From 8aed51592fbce06544270424dd239303564e9362 Mon Sep 17 00:00:00 2001 From: andrecs <12188364+andrecsilva@users.noreply.github.com> Date: Wed, 7 Aug 2024 10:04:12 -0300 Subject: [PATCH 1/8] Enabled Sonar SQL codemod in defaults --- .../src/main/java/io/codemodder/codemods/DefaultCodemods.java | 1 + 1 file changed, 1 insertion(+) diff --git a/core-codemods/src/main/java/io/codemodder/codemods/DefaultCodemods.java b/core-codemods/src/main/java/io/codemodder/codemods/DefaultCodemods.java index 2dde62f81..e7befa5b4 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/DefaultCodemods.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/DefaultCodemods.java @@ -57,6 +57,7 @@ public static List> asList() { SimplifyRestControllerAnnotationsCodemod.class, SubstituteReplaceAllCodemod.class, SonarXXECodemod.class, + SonarSQLInjectionCodemod.class, SQLParameterizerCodemod.class, SSRFCodemod.class, StackTraceExposureCodemod.class, From 33f495a10c559f5ac3c1c6b64f8e7a55f922d66c Mon Sep 17 00:00:00 2001 From: andrecs <12188364+andrecsilva@users.noreply.github.com> Date: Fri, 9 Aug 2024 10:37:28 -0300 Subject: [PATCH 2/8] Initial changes to includes/excludes --- .../integration/WebGoat822Test.java | 33 +++++++++++++- .../src/main/java/io/codemodder/CLI.java | 18 +++++--- .../main/java/io/codemodder/CodeChanger.java | 5 +++ .../java/io/codemodder/CodemodLoader.java | 2 + .../CompositeJavaParserChanger.java | 6 +++ .../io/codemodder/DefaultCodemodExecutor.java | 11 +++-- .../java/io/codemodder/IncludesExcludes.java | 1 + .../codemodder/IncludesExcludesPattern.java | 43 +++++++++++++++++++ .../io/codemodder/RawFileCodemodRunner.java | 13 +++++- .../javaparser/JavaParserChanger.java | 5 +++ .../javaparser/JavaParserCodemodRunner.java | 12 +++++- .../src/test/java/io/codemodder/CLITest.java | 5 +++ .../JavaParserCodemodRunnerTest.java | 4 ++ .../providers/sonar/DefaultRuleFinding.java | 15 ++++--- .../providers/sonar/RuleFinding.java | 3 ++ .../SonarRemediatingJavaParserChanger.java | 10 +++++ 16 files changed, 168 insertions(+), 18 deletions(-) create mode 100644 framework/codemodder-base/src/main/java/io/codemodder/IncludesExcludesPattern.java diff --git a/core-codemods/src/intTest/java/io/codemodder/integration/WebGoat822Test.java b/core-codemods/src/intTest/java/io/codemodder/integration/WebGoat822Test.java index 3254dfcfc..b5a515efc 100644 --- a/core-codemods/src/intTest/java/io/codemodder/integration/WebGoat822Test.java +++ b/core-codemods/src/intTest/java/io/codemodder/integration/WebGoat822Test.java @@ -26,6 +26,29 @@ final class WebGoat822Test extends GitRepositoryTest { super("https://github.com/WebGoat/WebGoat", "main", "e75cfbeb110e3d3a2ca3c8fee2754992d89c419d"); } + private static final String testPathIncludes = + "**.java," + + "**/*.java," + + "pom.xml," + + "**/pom.xml," + + "**.jsp," + + "**/*.jsp," + + "web.xml," + + "**/web.xml," + + ".github/workflows/*.yml," + + ".github/workflows/*.yaml"; + + private static final String testPathExcludes = + "**/test/**," + + "**/testFixtures/**," + + "**/*Test.java," + + "**/intTest/**," + + "**/tests/**," + + "**/target/**," + + "**/build/**," + + "**/.mvn/**," + + ".mvn/**"; + @Test void it_injects_dependency_even_when_no_poms_included() throws Exception { @@ -83,7 +106,13 @@ void it_injects_dependency_even_when_no_poms_included() throws Exception { void it_transforms_webgoat_normally() throws Exception { DefaultCodemods.main( new String[] { - "--output", outputFile.getPath(), "--verbose", "--dont-exit", repoDir.getPath() + "--output", + outputFile.getPath(), + "--verbose", + "--dont-exit", + repoDir.getPath(), + "--path-include=" + testPathIncludes, + "--path-exclude=" + testPathExcludes, }); ObjectMapper objectMapper = new ObjectMapper(); @@ -121,6 +150,8 @@ void it_transforms_webgoat_with_codeql() throws Exception { "--sarif", "src/test/resources/webgoat_v8.2.2_codeql.sarif", "--dont-exit", + "--path-include=" + testPathIncludes, + "--path-exclude=" + testPathExcludes, repoDir.getPath() }); diff --git a/framework/codemodder-base/src/main/java/io/codemodder/CLI.java b/framework/codemodder-base/src/main/java/io/codemodder/CLI.java index 6347bdf81..e9d5daecc 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/CLI.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/CLI.java @@ -344,16 +344,24 @@ public Integer call() throws IOException { // get path includes/excludes List pathIncludes = this.pathIncludes; + List pathExcludes = this.pathExcludes; + + // If no path includes/excludes were specified, relegate filtering to each codemod + // This is indicated by the MatchesEverything type of IncludesExcludes + boolean useGlobalIncludesExcludes = pathIncludes != null || pathExcludes != null; + IncludesExcludes includesExcludes = IncludesExcludes.any(); if (pathIncludes == null) { - pathIncludes = defaultPathIncludes; + pathIncludes = List.of("**"); } - List pathExcludes = this.pathExcludes; if (pathExcludes == null) { - pathExcludes = defaultPathExcludes; + pathExcludes = List.of(); + } + + if (useGlobalIncludesExcludes) { + includesExcludes = + IncludesExcludes.withSettings(projectDirectory, pathIncludes, pathExcludes); } - IncludesExcludes includesExcludes = - IncludesExcludes.withSettings(projectDirectory, pathIncludes, pathExcludes); log.debug("including paths: {}", pathIncludes); log.debug("excluding paths: {}", pathExcludes); diff --git a/framework/codemodder-base/src/main/java/io/codemodder/CodeChanger.java b/framework/codemodder-base/src/main/java/io/codemodder/CodeChanger.java index 141788463..4e8386b17 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/CodeChanger.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/CodeChanger.java @@ -22,6 +22,11 @@ public interface CodeChanger { /** A description of an individual change made by this codemod. */ String getIndividualChangeDescription(final Path filePath, final CodemodChange change); + /** A list of paths requested or rejected by the codemod * */ + default IncludesExcludesPattern getIncludesExcludesPattern() { + return IncludesExcludesPattern.getAnyMatcher(); + } + /** * A lifecycle event that is called before any files are processed. This is a good place to short * circuit if you don't have the necessary resources (e.g., SARIF). diff --git a/framework/codemodder-base/src/main/java/io/codemodder/CodemodLoader.java b/framework/codemodder-base/src/main/java/io/codemodder/CodemodLoader.java index dbc6daf11..caf36f587 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/CodemodLoader.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/CodemodLoader.java @@ -108,6 +108,8 @@ public CodemodLoader( allModules.addAll(modules); } + // TODO add interested files in each provider + // record which changers are associated with which codemod ids final List codemods = new ArrayList<>(); diff --git a/framework/codemodder-base/src/main/java/io/codemodder/CompositeJavaParserChanger.java b/framework/codemodder-base/src/main/java/io/codemodder/CompositeJavaParserChanger.java index 31d7462f0..83d38448a 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/CompositeJavaParserChanger.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/CompositeJavaParserChanger.java @@ -29,6 +29,12 @@ protected CompositeJavaParserChanger(final JavaParserChanger... changers) { this.changers = Arrays.asList(Objects.requireNonNull(changers)); } + @Override + public IncludesExcludesPattern getIncludesExcludesPattern() { + // The first changer will dictate which files this composition accepts + return changers.get(0).getIncludesExcludesPattern(); + } + @Override public CodemodFileScanningResult visit( final CodemodInvocationContext context, final CompilationUnit cu) { diff --git a/framework/codemodder-base/src/main/java/io/codemodder/DefaultCodemodExecutor.java b/framework/codemodder-base/src/main/java/io/codemodder/DefaultCodemodExecutor.java index b4e26c4ef..64380f5ac 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/DefaultCodemodExecutor.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/DefaultCodemodExecutor.java @@ -82,9 +82,14 @@ public CodeTFResult execute(final List filePaths) { if (codeChanger instanceof JavaParserChanger) { codemodRunner = new JavaParserCodemodRunner( - javaParserFacade, (JavaParserChanger) codeChanger, encodingDetector); + javaParserFacade, + (JavaParserChanger) codeChanger, + projectDir, + includesExcludes, + encodingDetector); } else if (codeChanger instanceof RawFileChanger) { - codemodRunner = new RawFileCodemodRunner((RawFileChanger) codeChanger); + codemodRunner = + new RawFileCodemodRunner((RawFileChanger) codeChanger, projectDir, includesExcludes); } else { throw new UnsupportedOperationException( "unsupported codeChanger type: " + codeChanger.getClass().getName()); @@ -95,7 +100,7 @@ public CodeTFResult execute(final List filePaths) { */ List codemodTargetFiles = filePaths.stream() - .filter(codemodRunner::supports) + .filter(p -> codemodRunner.supports(p)) .sorted() .limit(maxFiles != -1 ? maxFiles : Long.MAX_VALUE) .sorted() diff --git a/framework/codemodder-base/src/main/java/io/codemodder/IncludesExcludes.java b/framework/codemodder-base/src/main/java/io/codemodder/IncludesExcludes.java index f47fc4ff7..fa850c9fa 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/IncludesExcludes.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/IncludesExcludes.java @@ -166,6 +166,7 @@ private static boolean noPatternsSpecified( return includePatterns.isEmpty() && excludePatterns.isEmpty(); } + /** Matches every file, indicates that the codemods should use their own filtering. */ class MatchesEverything implements IncludesExcludes { @Override public boolean shouldInspect(final File file) { diff --git a/framework/codemodder-base/src/main/java/io/codemodder/IncludesExcludesPattern.java b/framework/codemodder-base/src/main/java/io/codemodder/IncludesExcludesPattern.java new file mode 100644 index 000000000..e294477f6 --- /dev/null +++ b/framework/codemodder-base/src/main/java/io/codemodder/IncludesExcludesPattern.java @@ -0,0 +1,43 @@ +package io.codemodder; + +import java.nio.file.Path; +import java.util.*; + +/** Holds includes and excludes patterns for files */ +public interface IncludesExcludesPattern { + + /** + * Returns an IncludesExcludes file matcher that matches files following the patterns against a + * root folder. + */ + public IncludesExcludes getRootedMatcher(final Path root); + + class Default implements IncludesExcludesPattern { + + private final List pathIncludes; + private final List pathExcludes; + + public Default(final List pathIncludes, final List pathExcludes) { + this.pathIncludes = pathIncludes; + this.pathExcludes = pathExcludes; + } + + @Override + public String toString() { + return "Includes: " + pathIncludes + "\nExcludes: " + pathExcludes; + } + + @Override + public IncludesExcludes getRootedMatcher(final Path root) { + return IncludesExcludes.withSettings(root.toFile(), pathIncludes, pathExcludes); + } + } + + public static IncludesExcludesPattern getJavaMatcher() { + return new Default(List.of("**.java"), List.of()); + } + + public static IncludesExcludesPattern getAnyMatcher() { + return new Default(List.of("**"), List.of()); + } +} diff --git a/framework/codemodder-base/src/main/java/io/codemodder/RawFileCodemodRunner.java b/framework/codemodder-base/src/main/java/io/codemodder/RawFileCodemodRunner.java index 6d2e5d7eb..e21bdfcc0 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/RawFileCodemodRunner.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/RawFileCodemodRunner.java @@ -12,14 +12,23 @@ final class RawFileCodemodRunner implements CodemodRunner { private final RawFileChanger changer; + private final IncludesExcludes rootedFileMatcher; - RawFileCodemodRunner(final RawFileChanger changer) { + RawFileCodemodRunner( + final RawFileChanger changer, + final Path projectDir, + final IncludesExcludes globalIncludesExcludes) { this.changer = Objects.requireNonNull(changer); + if (globalIncludesExcludes instanceof IncludesExcludes.MatchesEverything) { + this.rootedFileMatcher = changer.getIncludesExcludesPattern().getRootedMatcher(projectDir); + } else { + this.rootedFileMatcher = Objects.requireNonNull(globalIncludesExcludes); + } } @Override public boolean supports(final Path path) { - return true; + return rootedFileMatcher.shouldInspect(path.toFile()); } @Override diff --git a/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserChanger.java b/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserChanger.java index 02d9d027a..84319abf1 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserChanger.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserChanger.java @@ -38,6 +38,11 @@ public String getIndividualChangeDescription(final Path filePath, final CodemodC return reporter.getChange(filePath, change); } + @Override + public IncludesExcludesPattern getIncludesExcludesPattern() { + return IncludesExcludesPattern.getJavaMatcher(); + } + @Override public List getReferences() { return reporter.getReferences().stream().map(u -> new CodeTFReference(u, u)).toList(); diff --git a/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserCodemodRunner.java b/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserCodemodRunner.java index c698151b7..3a00ef24d 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserCodemodRunner.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserCodemodRunner.java @@ -21,19 +21,29 @@ public final class JavaParserCodemodRunner implements CodemodRunner { private final JavaParserChanger javaParserChanger; private final JavaParserFacade parser; private final EncodingDetector encodingDetector; + private final IncludesExcludes rootedFileMatcher; public JavaParserCodemodRunner( final JavaParserFacade parser, final JavaParserChanger javaParserChanger, + final Path projectDir, + final IncludesExcludes globalIncludesExcludes, final EncodingDetector encodingDetector) { this.parser = Objects.requireNonNull(parser); this.javaParserChanger = Objects.requireNonNull(javaParserChanger); this.encodingDetector = Objects.requireNonNull(encodingDetector); + if (globalIncludesExcludes instanceof IncludesExcludes.MatchesEverything) { + this.rootedFileMatcher = + javaParserChanger.getIncludesExcludesPattern().getRootedMatcher(projectDir); + } else { + this.rootedFileMatcher = Objects.requireNonNull(globalIncludesExcludes); + } } @Override public boolean supports(final Path path) { - return path.getFileName().toString().toLowerCase().endsWith(".java"); + return path.getFileName().toString().endsWith(".java") + && rootedFileMatcher.shouldInspect(path.toFile()); } @Override diff --git a/framework/codemodder-base/src/test/java/io/codemodder/CLITest.java b/framework/codemodder-base/src/test/java/io/codemodder/CLITest.java index 004cdfe54..a9814e130 100644 --- a/framework/codemodder-base/src/test/java/io/codemodder/CLITest.java +++ b/framework/codemodder-base/src/test/java/io/codemodder/CLITest.java @@ -242,6 +242,11 @@ public CodemodFileScanningResult visitFile(final CodemodInvocationContext contex } return CodemodFileScanningResult.none(); } + + @Override + public IncludesExcludesPattern getIncludesExcludesPattern() { + return new IncludesExcludesPattern.Default(List.of(), List.of("**/*Test.java")); + } } @Test diff --git a/framework/codemodder-base/src/test/java/io/codemodder/javaparser/JavaParserCodemodRunnerTest.java b/framework/codemodder-base/src/test/java/io/codemodder/javaparser/JavaParserCodemodRunnerTest.java index 35f706c24..03ec5c2fa 100644 --- a/framework/codemodder-base/src/test/java/io/codemodder/javaparser/JavaParserCodemodRunnerTest.java +++ b/framework/codemodder-base/src/test/java/io/codemodder/javaparser/JavaParserCodemodRunnerTest.java @@ -75,6 +75,8 @@ void setup(@TempDir Path tmpDir) { new JavaParserCodemodRunner( JavaParserFacade.from(JavaParser::new), updatesAllMethodNamesChanger, + tmpDir, + IncludesExcludes.any(), EncodingDetector.create()); this.tmpDir = tmpDir; } @@ -155,6 +157,8 @@ void it_composes_successfully() throws IOException { new UpdatesMethodNamesParserChanger(), new UpdatesClassNamesParserChanger(), new EmptyReporter()), + tmpDir, + IncludesExcludes.any(), EncodingDetector.create()); CodemodFileScanningResult result = runner.run(context); List changes = result.changes(); diff --git a/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/DefaultRuleFinding.java b/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/DefaultRuleFinding.java index b141f77b7..741356a98 100644 --- a/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/DefaultRuleFinding.java +++ b/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/DefaultRuleFinding.java @@ -2,12 +2,8 @@ import io.codemodder.sonar.model.SonarFinding; import java.nio.file.Path; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.Optional; +import java.util.*; +import java.util.stream.Stream; abstract class DefaultRuleFinding implements RuleFinding { @@ -37,4 +33,11 @@ public List getResultsByPath(final Path path) { public boolean hasResults() { return !findings.isEmpty(); } + + @Override + public List getPaths() { + var pathSet = new HashSet(); + Stream allFindings = findings.values().stream().flatMap(l -> l.stream()); + return allFindings.flatMap(f -> f.componentFileName().stream()).distinct().toList(); + } } diff --git a/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/RuleFinding.java b/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/RuleFinding.java index cfb01ab01..5b8def9fe 100644 --- a/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/RuleFinding.java +++ b/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/RuleFinding.java @@ -12,4 +12,7 @@ public interface RuleFinding { /** Whether any results are available. */ boolean hasResults(); + + /** Get all file paths that have any results. */ + List getPaths(); } diff --git a/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/SonarRemediatingJavaParserChanger.java b/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/SonarRemediatingJavaParserChanger.java index eba4768e6..6823fddec 100644 --- a/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/SonarRemediatingJavaParserChanger.java +++ b/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/SonarRemediatingJavaParserChanger.java @@ -2,18 +2,28 @@ import io.codemodder.CodemodReporterStrategy; import io.codemodder.FixOnlyCodeChanger; +import io.codemodder.IncludesExcludesPattern; import io.codemodder.javaparser.JavaParserChanger; +import java.util.List; /** Provides base functionality for making JavaParser-based remediation of Sonar results. */ public abstract class SonarRemediatingJavaParserChanger extends JavaParserChanger implements FixOnlyCodeChanger { private final boolean shouldRun; + private final IncludesExcludesPattern includesExcludesPattern; protected SonarRemediatingJavaParserChanger( final CodemodReporterStrategy reporter, final RuleFinding findings) { super(reporter); this.shouldRun = findings.hasResults(); + List allPathFindings = findings.getPaths(); + this.includesExcludesPattern = new IncludesExcludesPattern.Default(allPathFindings, List.of()); + } + + @Override + public IncludesExcludesPattern getIncludesExcludesPattern() { + return this.includesExcludesPattern; } @Override From c5b992454401f924197814a75298910f569a9be6 Mon Sep 17 00:00:00 2001 From: andrecs <12188364+andrecsilva@users.noreply.github.com> Date: Wed, 14 Aug 2024 08:43:28 -0300 Subject: [PATCH 3/8] Adapted remaining changers and bugfixes --- .../codemods/JSPScriptletXSSCodemod.java | 10 +++++ .../codemods/VerbTamperingCodemod.java | 5 +++ .../main/java/io/codemodder/CodeChanger.java | 5 ++- .../io/codemodder/DefaultCodemodExecutor.java | 15 ++++---- .../codemodder/IncludesExcludesPattern.java | 37 ++++++++++++++++--- .../io/codemodder/LazyLoadingRuleSarif.java | 8 ++++ .../main/java/io/codemodder/RuleSarif.java | 12 ++++++ .../SarifPluginJavaParserChanger.java | 9 +++++ .../codemodder/SarifPluginRawFileChanger.java | 5 +++ .../javaparser/JavaParserCodemodRunner.java | 1 + .../src/test/java/io/codemodder/CLITest.java | 3 +- .../sarif/appscan/AppScanRuleSarif.java | 6 +++ .../sarif/codeql/CodeQLRuleSarif.java | 29 +++++++++++++-- .../providers/sarif/pmd/PmdModule.java | 3 +- .../providers/sarif/pmd/PmdRuleSarif.java | 20 +++++++++- .../sarif/semgrep/SingleSemgrepRuleSarif.java | 35 ++++++++++++++++++ .../providers/sonar/DefaultRuleFinding.java | 5 ++- .../providers/sonar/RuleFinding.java | 3 +- .../sonar/SonarPluginJavaParserChanger.java | 7 ++++ .../SonarRemediatingJavaParserChanger.java | 6 +-- 20 files changed, 196 insertions(+), 28 deletions(-) diff --git a/core-codemods/src/main/java/io/codemodder/codemods/JSPScriptletXSSCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/JSPScriptletXSSCodemod.java index 3ba848585..fcbb00558 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/JSPScriptletXSSCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/JSPScriptletXSSCodemod.java @@ -2,6 +2,7 @@ import io.codemodder.*; import java.util.List; +import java.util.Set; import java.util.regex.Pattern; /** @@ -19,12 +20,16 @@ reviewGuidance = ReviewGuidance.MERGE_WITHOUT_REVIEW) public final class JSPScriptletXSSCodemod extends RegexFileChanger { + private IncludesExcludesPattern includesExcludesPattern; + public JSPScriptletXSSCodemod() { super( path -> path.getFileName().toString().toLowerCase().endsWith(".jsp"), scriptlet, true, List.of(DependencyGAV.OWASP_XSS_JAVA_ENCODER)); + this.includesExcludesPattern = + new IncludesExcludesPattern.Default(Set.of("**.[jJ][sS][pP]"), Set.of()); } @Override @@ -38,4 +43,9 @@ public String getReplacementFor(final String matchingSnippet) { Pattern.compile( "<%(\\s*)=(\\s*)request(\\s*).(\\s*)get((Header|Parameter)(\\s*)\\((\\s*)\".*\"(\\s*)\\)|QueryString\\((\\s*)\\))(\\s*)%>", Pattern.MULTILINE); + + @Override + public IncludesExcludesPattern getIncludesExcludesPattern() { + return includesExcludesPattern; + } } diff --git a/core-codemods/src/main/java/io/codemodder/codemods/VerbTamperingCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/VerbTamperingCodemod.java index 47823a05a..63149533c 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/VerbTamperingCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/VerbTamperingCodemod.java @@ -107,4 +107,9 @@ public String getIndividualChangeDescription(final Path filePath, final CodemodC public List getReferences() { return reporter.getReferences().stream().map(u -> new CodeTFReference(u, u)).toList(); } + + @Override + public IncludesExcludesPattern getIncludesExcludesPattern() { + return new IncludesExcludesPattern.Default(Set.of("{,**/}[wW][eE][bB].[xX][mM][lL]"), Set.of()); + } } diff --git a/framework/codemodder-base/src/main/java/io/codemodder/CodeChanger.java b/framework/codemodder-base/src/main/java/io/codemodder/CodeChanger.java index 4e8386b17..0d4c9a97e 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/CodeChanger.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/CodeChanger.java @@ -22,7 +22,10 @@ public interface CodeChanger { /** A description of an individual change made by this codemod. */ String getIndividualChangeDescription(final Path filePath, final CodemodChange change); - /** A list of paths requested or rejected by the codemod * */ + /** + * A list of paths requested or rejected by the codemod. Those patterns are treated as relative to + * the repository root. + */ default IncludesExcludesPattern getIncludesExcludesPattern() { return IncludesExcludesPattern.getAnyMatcher(); } diff --git a/framework/codemodder-base/src/main/java/io/codemodder/DefaultCodemodExecutor.java b/framework/codemodder-base/src/main/java/io/codemodder/DefaultCodemodExecutor.java index 64380f5ac..3aab72338 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/DefaultCodemodExecutor.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/DefaultCodemodExecutor.java @@ -100,7 +100,7 @@ public CodeTFResult execute(final List filePaths) { */ List codemodTargetFiles = filePaths.stream() - .filter(p -> codemodRunner.supports(p)) + .filter(codemodRunner::supports) .sorted() .limit(maxFiles != -1 ? maxFiles : Long.MAX_VALUE) .sorted() @@ -371,13 +371,12 @@ private CodemodPackageUpdateResult addPackages( dependencies.stream() .filter(d -> !skippedDependencies.contains(d)) .forEach( - dep -> { - pkgActions.add( - new CodeTFPackageAction( - CodeTFPackageAction.CodeTFPackageActionType.ADD, - CodeTFPackageAction.CodeTFPackageActionResult.FAILED, - toPackageUrl(dep))); - }); + dep -> + pkgActions.add( + new CodeTFPackageAction( + CodeTFPackageAction.CodeTFPackageActionType.ADD, + CodeTFPackageAction.CodeTFPackageActionResult.FAILED, + toPackageUrl(dep)))); return CodemodPackageUpdateResult.from(pkgActions, pkgChanges, unscannableFiles); } diff --git a/framework/codemodder-base/src/main/java/io/codemodder/IncludesExcludesPattern.java b/framework/codemodder-base/src/main/java/io/codemodder/IncludesExcludesPattern.java index e294477f6..a7f8c8db0 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/IncludesExcludesPattern.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/IncludesExcludesPattern.java @@ -14,10 +14,10 @@ public interface IncludesExcludesPattern { class Default implements IncludesExcludesPattern { - private final List pathIncludes; - private final List pathExcludes; + private final Set pathIncludes; + private final Set pathExcludes; - public Default(final List pathIncludes, final List pathExcludes) { + public Default(final Set pathIncludes, final Set pathExcludes) { this.pathIncludes = pathIncludes; this.pathExcludes = pathExcludes; } @@ -29,15 +29,40 @@ public String toString() { @Override public IncludesExcludes getRootedMatcher(final Path root) { - return IncludesExcludes.withSettings(root.toFile(), pathIncludes, pathExcludes); + return IncludesExcludes.withSettings( + root.toFile(), pathIncludes.stream().toList(), pathExcludes.stream().toList()); + } + } + + final class JavaMatcherSingleton { + private static IncludesExcludesPattern singleton; + + public static IncludesExcludesPattern getInstance() { + if (singleton == null) { + singleton = + new IncludesExcludesPattern.Default( + Set.of("**.[jJ][aA][vV][Aa]"), Collections.emptySet()); + } + return singleton; + } + } + + final class AnySingleton { + private static IncludesExcludesPattern singleton; + + public static IncludesExcludesPattern getInstance() { + if (singleton == null) { + singleton = new IncludesExcludesPattern.Default(Set.of("**"), Collections.emptySet()); + } + return singleton; } } public static IncludesExcludesPattern getJavaMatcher() { - return new Default(List.of("**.java"), List.of()); + return JavaMatcherSingleton.getInstance(); } public static IncludesExcludesPattern getAnyMatcher() { - return new Default(List.of("**"), List.of()); + return AnySingleton.getInstance(); } } diff --git a/framework/codemodder-base/src/main/java/io/codemodder/LazyLoadingRuleSarif.java b/framework/codemodder-base/src/main/java/io/codemodder/LazyLoadingRuleSarif.java index 4ff624b64..9f326ebf5 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/LazyLoadingRuleSarif.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/LazyLoadingRuleSarif.java @@ -6,6 +6,7 @@ import java.nio.file.Path; import java.util.List; import java.util.Objects; +import java.util.Set; import javax.inject.Provider; /** @@ -60,4 +61,11 @@ public String getDriver() { checkInitialized(); return ruleSarif.getDriver(); } + + @Override + public Set getPaths() { + // Lazy loading sarif files are generated by running a tool before inspecting the tree + // Thus, it doesn't have a list of files beforehand and, as such, should accept any file + return Set.of("**"); + } } diff --git a/framework/codemodder-base/src/main/java/io/codemodder/RuleSarif.java b/framework/codemodder-base/src/main/java/io/codemodder/RuleSarif.java index 07de370ca..7c43c5903 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/RuleSarif.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/RuleSarif.java @@ -5,6 +5,7 @@ import com.contrastsecurity.sarif.SarifSchema210; import java.nio.file.Path; import java.util.List; +import java.util.Set; /** Defines a model for interacting with SARIF. */ public interface RuleSarif { @@ -37,6 +38,12 @@ public interface RuleSarif { /** Returns the tool driver that produced this SARIF. */ String getDriver(); + /** + * Returns all file paths with results for the rule. Those paths are relative to the repository + * root. + */ + Set getPaths(); + /** An empty implementation of {@link RuleSarif} for binding codemods with no SARIF results. */ final class EmptyRuleSarif implements RuleSarif { @@ -64,5 +71,10 @@ public String getDriver() { public SarifSchema210 rawDocument() { return new SarifSchema210(); } + + @Override + public Set getPaths() { + return Set.of(); + } } } diff --git a/framework/codemodder-base/src/main/java/io/codemodder/SarifPluginJavaParserChanger.java b/framework/codemodder-base/src/main/java/io/codemodder/SarifPluginJavaParserChanger.java index e65e31382..518abb689 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/SarifPluginJavaParserChanger.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/SarifPluginJavaParserChanger.java @@ -13,6 +13,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; +import java.util.Set; /** * Provides base functionality for making JavaParser-based changes based on results found by a sarif @@ -24,6 +25,7 @@ public abstract class SarifPluginJavaParserChanger extends JavaP private final Class nodeType; private final SourceCodeRegionExtractor regionExtractor; private final RegionNodeMatcher regionNodeMatcher; + private final IncludesExcludesPattern includesExcludesPattern; protected SarifPluginJavaParserChanger( final RuleSarif sarif, final Class nodeType) { @@ -77,6 +79,7 @@ protected SarifPluginJavaParserChanger( this.nodeType = Objects.requireNonNull(nodeType); this.regionExtractor = Objects.requireNonNull(regionExtractor); this.regionNodeMatcher = Objects.requireNonNull(regionNodeMatcher); + this.includesExcludesPattern = new IncludesExcludesPattern.Default(sarif.getPaths(), Set.of()); } protected SarifPluginJavaParserChanger( @@ -90,6 +93,7 @@ protected SarifPluginJavaParserChanger( this.nodeType = Objects.requireNonNull(nodeType); this.regionExtractor = Objects.requireNonNull(regionExtractor); this.regionNodeMatcher = Objects.requireNonNull(regionNodeMatcher); + this.includesExcludesPattern = new IncludesExcludesPattern.Default(sarif.getPaths(), Set.of()); } public CodemodFileScanningResult visit( @@ -183,4 +187,9 @@ public boolean shouldRun() { */ public abstract ChangesResult onResultFound( CodemodInvocationContext context, CompilationUnit cu, T node, Result result); + + @Override + public IncludesExcludesPattern getIncludesExcludesPattern() { + return includesExcludesPattern; + } } diff --git a/framework/codemodder-base/src/main/java/io/codemodder/SarifPluginRawFileChanger.java b/framework/codemodder-base/src/main/java/io/codemodder/SarifPluginRawFileChanger.java index 21a7ef809..990128b22 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/SarifPluginRawFileChanger.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/SarifPluginRawFileChanger.java @@ -2,20 +2,25 @@ import com.contrastsecurity.sarif.Result; import java.util.List; +import java.util.Set; /** A {@link RawFileChanger} bundled with a {@link RuleSarif}. */ public abstract class SarifPluginRawFileChanger extends RawFileChanger { private final RuleSarif sarif; + private final IncludesExcludesPattern includesExcludesPattern; + protected SarifPluginRawFileChanger(final RuleSarif sarif) { this.sarif = sarif; + this.includesExcludesPattern = new IncludesExcludesPattern.Default(sarif.getPaths(), Set.of()); } protected SarifPluginRawFileChanger( final RuleSarif sarif, final CodemodReporterStrategy reporter) { super(reporter); this.sarif = sarif; + this.includesExcludesPattern = new IncludesExcludesPattern.Default(sarif.getPaths(), Set.of()); } @Override diff --git a/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserCodemodRunner.java b/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserCodemodRunner.java index 3a00ef24d..0a6f1bedd 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserCodemodRunner.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserCodemodRunner.java @@ -32,6 +32,7 @@ public JavaParserCodemodRunner( this.parser = Objects.requireNonNull(parser); this.javaParserChanger = Objects.requireNonNull(javaParserChanger); this.encodingDetector = Objects.requireNonNull(encodingDetector); + // If no global includes/excludes are specified, each codemod will decide which files to inspect if (globalIncludesExcludes instanceof IncludesExcludes.MatchesEverything) { this.rootedFileMatcher = javaParserChanger.getIncludesExcludesPattern().getRootedMatcher(projectDir); diff --git a/framework/codemodder-base/src/test/java/io/codemodder/CLITest.java b/framework/codemodder-base/src/test/java/io/codemodder/CLITest.java index a9814e130..eaf1c26c9 100644 --- a/framework/codemodder-base/src/test/java/io/codemodder/CLITest.java +++ b/framework/codemodder-base/src/test/java/io/codemodder/CLITest.java @@ -24,6 +24,7 @@ import java.time.Clock; import java.util.List; import java.util.Optional; +import java.util.Set; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -245,7 +246,7 @@ public CodemodFileScanningResult visitFile(final CodemodInvocationContext contex @Override public IncludesExcludesPattern getIncludesExcludesPattern() { - return new IncludesExcludesPattern.Default(List.of(), List.of("**/*Test.java")); + return new IncludesExcludesPattern.Default(Set.of(), Set.of("**/*Test.java")); } } diff --git a/plugins/codemodder-plugin-appscan/src/main/java/io/codemodder/providers/sarif/appscan/AppScanRuleSarif.java b/plugins/codemodder-plugin-appscan/src/main/java/io/codemodder/providers/sarif/appscan/AppScanRuleSarif.java index a46cb928d..29577bd10 100644 --- a/plugins/codemodder-plugin-appscan/src/main/java/io/codemodder/providers/sarif/appscan/AppScanRuleSarif.java +++ b/plugins/codemodder-plugin-appscan/src/main/java/io/codemodder/providers/sarif/appscan/AppScanRuleSarif.java @@ -7,6 +7,7 @@ import java.io.UncheckedIOException; import java.nio.file.Path; import java.util.*; +import java.util.stream.Collectors; /** A {@link RuleSarif} for AppScan results. */ final class AppScanRuleSarif implements RuleSarif { @@ -92,6 +93,11 @@ public List getResultsByLocationPath(final Path path) { .toList()); } + @Override + public Set getPaths() { + return locations.stream().collect(Collectors.toSet()); + } + @Override public String getDriver() { return toolName; diff --git a/plugins/codemodder-plugin-codeql/src/main/java/io/codemodder/providers/sarif/codeql/CodeQLRuleSarif.java b/plugins/codemodder-plugin-codeql/src/main/java/io/codemodder/providers/sarif/codeql/CodeQLRuleSarif.java index 0b89ba5eb..203cacb1a 100644 --- a/plugins/codemodder-plugin-codeql/src/main/java/io/codemodder/providers/sarif/codeql/CodeQLRuleSarif.java +++ b/plugins/codemodder-plugin-codeql/src/main/java/io/codemodder/providers/sarif/codeql/CodeQLRuleSarif.java @@ -9,10 +9,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Objects; +import java.util.*; import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -24,6 +21,7 @@ public final class CodeQLRuleSarif implements RuleSarif { private final String ruleId; private final Map> resultsCache; private final Path repositoryRoot; + private Set relativeLocations; public CodeQLRuleSarif( final String ruleId, final SarifSchema210 sarif, final CodeDirectory codeDirectory) { @@ -31,6 +29,7 @@ public CodeQLRuleSarif( this.ruleId = Objects.requireNonNull(ruleId); this.repositoryRoot = codeDirectory.asPath(); this.resultsCache = new HashMap<>(); + this.relativeLocations = null; } private String extractRuleId(final Result result, final Run run) { @@ -96,6 +95,28 @@ public List getResultsByLocationPath(final Path path) { return results; } + @Override + public Set getPaths() { + if (relativeLocations == null) { + relativeLocations = + sarif.getRuns().stream() + .flatMap( + run -> + run.getResults().stream() + .filter(result -> ruleId.equals(extractRuleId(result, run)))) + .map( + result -> + result + .getLocations() + .get(0) + .getPhysicalLocation() + .getArtifactLocation() + .getUri()) + .collect(Collectors.toSet()); + } + return relativeLocations; + } + @Override public SarifSchema210 rawDocument() { return sarif; diff --git a/plugins/codemodder-plugin-pmd/src/main/java/io/codemodder/providers/sarif/pmd/PmdModule.java b/plugins/codemodder-plugin-pmd/src/main/java/io/codemodder/providers/sarif/pmd/PmdModule.java index d52d65f41..51afaa1ca 100644 --- a/plugins/codemodder-plugin-pmd/src/main/java/io/codemodder/providers/sarif/pmd/PmdModule.java +++ b/plugins/codemodder-plugin-pmd/src/main/java/io/codemodder/providers/sarif/pmd/PmdModule.java @@ -127,7 +127,8 @@ protected void configure() { resultsByFile.computeIfAbsent(normalizedFilePath, k -> new ArrayList<>()); resultsForFile.add(result); } - return new PmdRuleSarif(trimmedRuleId, rawSarifFromRun, resultsByFile); + return new PmdRuleSarif( + trimmedRuleId, rawSarifFromRun, resultsByFile, this.codeDirectory); }); this.bind(RuleSarif.class).annotatedWith(scanTarget.pmdScan).toInstance(sarif); diff --git a/plugins/codemodder-plugin-pmd/src/main/java/io/codemodder/providers/sarif/pmd/PmdRuleSarif.java b/plugins/codemodder-plugin-pmd/src/main/java/io/codemodder/providers/sarif/pmd/PmdRuleSarif.java index 67db249b6..c2058f0f6 100644 --- a/plugins/codemodder-plugin-pmd/src/main/java/io/codemodder/providers/sarif/pmd/PmdRuleSarif.java +++ b/plugins/codemodder-plugin-pmd/src/main/java/io/codemodder/providers/sarif/pmd/PmdRuleSarif.java @@ -8,20 +8,27 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; final class PmdRuleSarif implements RuleSarif { private final SarifSchema210 sarif; private final String ruleId; private final Map> resultsByFile; + private final Path repositoryRoot; + private Set relativeLocations; PmdRuleSarif( final String ruleId, final SarifSchema210 sarif, - final Map> resultsByFile) { + final Map> resultsByFile, + final Path repositoryRoot) { this.ruleId = Objects.requireNonNull(ruleId); this.sarif = Objects.requireNonNull(sarif); this.resultsByFile = Objects.requireNonNull(resultsByFile); + this.repositoryRoot = repositoryRoot; + this.relativeLocations = null; } @Override @@ -53,4 +60,15 @@ public String getDriver() { } static final String toolName = "pmd"; + + @Override + public Set getPaths() { + if (this.relativeLocations == null) { + relativeLocations = + resultsByFile.keySet().stream() + .map(s -> repositoryRoot.relativize(Path.of(s)).toString()) + .collect(Collectors.toSet()); + } + return relativeLocations; + } } diff --git a/plugins/codemodder-plugin-semgrep/src/main/java/io/codemodder/providers/sarif/semgrep/SingleSemgrepRuleSarif.java b/plugins/codemodder-plugin-semgrep/src/main/java/io/codemodder/providers/sarif/semgrep/SingleSemgrepRuleSarif.java index 2dc463f4f..601c0255e 100644 --- a/plugins/codemodder-plugin-semgrep/src/main/java/io/codemodder/providers/sarif/semgrep/SingleSemgrepRuleSarif.java +++ b/plugins/codemodder-plugin-semgrep/src/main/java/io/codemodder/providers/sarif/semgrep/SingleSemgrepRuleSarif.java @@ -8,6 +8,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.*; +import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -24,6 +25,7 @@ final class SingleSemgrepRuleSarif implements RuleSarif { private final String ruleId; private final Map> resultsCache; private final Path repositoryRoot; + private Set relativeLocations; SingleSemgrepRuleSarif( final String ruleId, final SarifSchema210 sarif, final Path codeDirectory) { @@ -31,6 +33,7 @@ final class SingleSemgrepRuleSarif implements RuleSarif { this.ruleId = Objects.requireNonNull(ruleId); this.repositoryRoot = Objects.requireNonNull(codeDirectory); this.resultsCache = new HashMap<>(); + this.relativeLocations = null; } @Override @@ -89,6 +92,38 @@ public List getResultsByLocationPath(final Path path) { return results; } + @Override + public Set getPaths() { + if (relativeLocations == null) { + relativeLocations = + sarif.getRuns().get(0).getResults().stream() + /* + * The default Semgrep rules have a rule id reported that is what you'd expect. When you run + * your own custom rules locally, they'll contain part of the file system path to the rule. + * + * Because this provides support for both types, we need this check to account for which type + * of rule id we're dealing with. + */ + .filter( + result -> + result.getRuleId().endsWith("." + ruleId) + || result.getRuleId().equals(ruleId)) + .map( + result -> + result + .getLocations() + .get(0) + .getPhysicalLocation() + .getArtifactLocation() + .getUri()) + .map(s -> Path.of(s)) + .map(p -> p.startsWith(repositoryRoot) ? repositoryRoot.relativize(p) : p) + .map(p -> p.toString()) + .collect(Collectors.toSet()); + } + return relativeLocations; + } + @Override public String getDriver() { return sarif.getRuns().get(0).getTool().getDriver().getName(); diff --git a/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/DefaultRuleFinding.java b/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/DefaultRuleFinding.java index 741356a98..149b1f8d4 100644 --- a/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/DefaultRuleFinding.java +++ b/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/DefaultRuleFinding.java @@ -3,6 +3,7 @@ import io.codemodder.sonar.model.SonarFinding; import java.nio.file.Path; import java.util.*; +import java.util.stream.Collectors; import java.util.stream.Stream; abstract class DefaultRuleFinding implements RuleFinding { @@ -35,9 +36,9 @@ public boolean hasResults() { } @Override - public List getPaths() { + public Set getPaths() { var pathSet = new HashSet(); Stream allFindings = findings.values().stream().flatMap(l -> l.stream()); - return allFindings.flatMap(f -> f.componentFileName().stream()).distinct().toList(); + return allFindings.flatMap(f -> f.componentFileName().stream()).collect(Collectors.toSet()); } } diff --git a/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/RuleFinding.java b/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/RuleFinding.java index 5b8def9fe..553946591 100644 --- a/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/RuleFinding.java +++ b/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/RuleFinding.java @@ -3,6 +3,7 @@ import io.codemodder.sonar.model.SonarFinding; import java.nio.file.Path; import java.util.List; +import java.util.Set; /** A view of the Sonar findings results file for a given rule. */ public interface RuleFinding { @@ -14,5 +15,5 @@ public interface RuleFinding { boolean hasResults(); /** Get all file paths that have any results. */ - List getPaths(); + Set getPaths(); } diff --git a/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/SonarPluginJavaParserChanger.java b/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/SonarPluginJavaParserChanger.java index 59c476158..7e8cef2df 100644 --- a/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/SonarPluginJavaParserChanger.java +++ b/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/SonarPluginJavaParserChanger.java @@ -11,6 +11,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; +import java.util.Set; /** Provides base functionality for making JavaParser-based changes based on Sonar results. */ public abstract class SonarPluginJavaParserChanger @@ -22,6 +23,8 @@ public abstract class SonarPluginJavaParserChanger ruleFinding, final Class nodeType, @@ -31,6 +34,8 @@ protected SonarPluginJavaParserChanger( this.nodeType = Objects.requireNonNull(nodeType); this.regionNodeMatcher = regionNodeMatcher; this.nodeCollector = nodeCollector; + this.includesExcludesPattern = + new IncludesExcludesPattern.Default(ruleFinding.getPaths(), Set.of()); } protected SonarPluginJavaParserChanger( @@ -48,6 +53,8 @@ protected SonarPluginJavaParserChanger( this.nodeType = Objects.requireNonNull(nodeType); this.regionNodeMatcher = regionNodeMatcher; this.nodeCollector = NodeCollector.ALL_FROM_TYPE; + this.includesExcludesPattern = + new IncludesExcludesPattern.Default(ruleFinding.getPaths(), Set.of()); } @Override diff --git a/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/SonarRemediatingJavaParserChanger.java b/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/SonarRemediatingJavaParserChanger.java index 6823fddec..a9980e83c 100644 --- a/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/SonarRemediatingJavaParserChanger.java +++ b/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/SonarRemediatingJavaParserChanger.java @@ -4,7 +4,7 @@ import io.codemodder.FixOnlyCodeChanger; import io.codemodder.IncludesExcludesPattern; import io.codemodder.javaparser.JavaParserChanger; -import java.util.List; +import java.util.Set; /** Provides base functionality for making JavaParser-based remediation of Sonar results. */ public abstract class SonarRemediatingJavaParserChanger extends JavaParserChanger @@ -17,8 +17,8 @@ protected SonarRemediatingJavaParserChanger( final CodemodReporterStrategy reporter, final RuleFinding findings) { super(reporter); this.shouldRun = findings.hasResults(); - List allPathFindings = findings.getPaths(); - this.includesExcludesPattern = new IncludesExcludesPattern.Default(allPathFindings, List.of()); + Set allPathFindings = findings.getPaths(); + this.includesExcludesPattern = new IncludesExcludesPattern.Default(allPathFindings, Set.of()); } @Override From 6b6639329d592426df15a293e20432f9a55ac50a Mon Sep 17 00:00:00 2001 From: andrecs <12188364+andrecsilva@users.noreply.github.com> Date: Fri, 16 Aug 2024 08:00:13 -0300 Subject: [PATCH 4/8] Refactored many codemod checks into supports method * Reverted changes for includes/excludes in sarif files --- .../codemods/AddMissingI18nCodemod.java | 10 +++--- .../codemods/JSPScriptletXSSCodemod.java | 12 ++++--- .../codemods/MavenSecureURLCodemod.java | 2 +- .../SpringAbsoluteCookieTimeoutCodemod.java | 9 +++-- .../codemods/VerbTamperingCodemod.java | 8 +++-- .../main/java/io/codemodder/CodeChanger.java | 11 ++++-- .../java/io/codemodder/CodemodLoader.java | 2 -- .../CompositeJavaParserChanger.java | 11 +++--- .../io/codemodder/LazyLoadingRuleSarif.java | 8 ----- .../io/codemodder/RawFileCodemodRunner.java | 5 +-- .../java/io/codemodder/RegexFileChanger.java | 8 ----- .../main/java/io/codemodder/RuleSarif.java | 12 ------- .../SarifPluginJavaParserChanger.java | 22 ++++-------- .../codemodder/SarifPluginRawFileChanger.java | 17 ++++----- .../javaparser/JavaParserChanger.java | 5 +++ .../javaparser/JavaParserCodemodRunner.java | 7 +--- .../src/test/java/io/codemodder/CLITest.java | 5 +++ .../java/io/codemodder/CodemodLoaderTest.java | 25 +++++++++++++ .../sarif/appscan/AppScanRuleSarif.java | 6 ---- .../sarif/appscan/AppScanModuleTest.java | 5 +++ .../sarif/codeql/CodeQLRuleSarif.java | 24 ------------- .../providers/sarif/pmd/PmdRuleSarif.java | 15 -------- .../sarif/semgrep/SingleSemgrepRuleSarif.java | 35 ------------------- .../sarif/semgrep/SemgrepModuleTest.java | 12 ++++++- .../BindsToIncorrectObject.java | 5 +++ .../providers/sonar/DefaultRuleFinding.java | 9 ----- .../providers/sonar/RuleFinding.java | 4 --- .../sonar/SonarPluginJavaParserChanger.java | 26 +++++--------- .../SonarRemediatingJavaParserChanger.java | 15 ++------ 29 files changed, 122 insertions(+), 213 deletions(-) diff --git a/core-codemods/src/main/java/io/codemodder/codemods/AddMissingI18nCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/AddMissingI18nCodemod.java index 8748eee69..0582dc7dc 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/AddMissingI18nCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/AddMissingI18nCodemod.java @@ -53,16 +53,18 @@ private List putInPreferredOrder(final List languages) { return orderedLanguages; } + @Override + public boolean supports(final Path file) { + return getPropertyFilePrefix(file.getFileName().toString()).isPresent(); + } + @Override public CodemodFileScanningResult visitFile(final CodemodInvocationContext context) throws IOException { Path path = context.path(); String fileName = path.getFileName().toString(); + // The supports check will guarantee that this won't be empty Optional prefix = getPropertyFilePrefix(fileName); - if (prefix.isEmpty()) { - // doesn't look like a i18n properties file - return CodemodFileScanningResult.none(); - } return doVisitFile(context, path, prefix.get()); } diff --git a/core-codemods/src/main/java/io/codemodder/codemods/JSPScriptletXSSCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/JSPScriptletXSSCodemod.java index fcbb00558..7c286cbfd 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/JSPScriptletXSSCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/JSPScriptletXSSCodemod.java @@ -1,6 +1,7 @@ package io.codemodder.codemods; import io.codemodder.*; +import java.nio.file.Path; import java.util.List; import java.util.Set; import java.util.regex.Pattern; @@ -23,11 +24,7 @@ public final class JSPScriptletXSSCodemod extends RegexFileChanger { private IncludesExcludesPattern includesExcludesPattern; public JSPScriptletXSSCodemod() { - super( - path -> path.getFileName().toString().toLowerCase().endsWith(".jsp"), - scriptlet, - true, - List.of(DependencyGAV.OWASP_XSS_JAVA_ENCODER)); + super(scriptlet, true, List.of(DependencyGAV.OWASP_XSS_JAVA_ENCODER)); this.includesExcludesPattern = new IncludesExcludesPattern.Default(Set.of("**.[jJ][sS][pP]"), Set.of()); } @@ -39,6 +36,11 @@ public String getReplacementFor(final String matchingSnippet) { return "<%=org.owasp.encoder.Encode.forHtml(" + codeWithinScriptlet + ")%>"; } + @Override + public boolean supports(Path file) { + return file.getFileName().toString().toLowerCase().endsWith(".jsp"); + } + private static final Pattern scriptlet = Pattern.compile( "<%(\\s*)=(\\s*)request(\\s*).(\\s*)get((Header|Parameter)(\\s*)\\((\\s*)\".*\"(\\s*)\\)|QueryString\\((\\s*)\\))(\\s*)%>", 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 e0ed30284..e44121701 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/MavenSecureURLCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/MavenSecureURLCodemod.java @@ -126,7 +126,7 @@ private CodemodFileScanningResult processXml(final Path file, final List } /* - * Change contents of the {@code url} tag if it it uses an insecure protocol. + * Change contents of the {@code url} tag if it uses an insecure protocol. */ private static void handle( final XMLEventReader xmlEventReader, diff --git a/core-codemods/src/main/java/io/codemodder/codemods/SpringAbsoluteCookieTimeoutCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/SpringAbsoluteCookieTimeoutCodemod.java index f794eb317..cd5f67c19 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/SpringAbsoluteCookieTimeoutCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/SpringAbsoluteCookieTimeoutCodemod.java @@ -46,13 +46,16 @@ public SpringAbsoluteCookieTimeoutCodemod( this.safeDuration = parseExistingValueFromLine(count, units); } + @Override + public boolean supports(final Path file) { + return "application.properties".equalsIgnoreCase(file.getFileName().toString()); + } + @Override public CodemodFileScanningResult visitFile(final CodemodInvocationContext context) throws IOException { Path path = context.path(); - if (!"application.properties".equalsIgnoreCase(path.getFileName().toString())) { - return CodemodFileScanningResult.none(); - } else if (!inExpectedDir(context.codeDirectory().asPath().relativize(path))) { + if (!inExpectedDir(context.codeDirectory().asPath().relativize(path))) { return CodemodFileScanningResult.none(); } diff --git a/core-codemods/src/main/java/io/codemodder/codemods/VerbTamperingCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/VerbTamperingCodemod.java index 63149533c..340dc2c9c 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/VerbTamperingCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/VerbTamperingCodemod.java @@ -36,9 +36,6 @@ public VerbTamperingCodemod(final XPathStreamProcessor processor) { public CodemodFileScanningResult visitFile(final CodemodInvocationContext context) throws IOException { Path file = context.path(); - if (!"web.xml".equalsIgnoreCase(file.getFileName().toString())) { - return CodemodFileScanningResult.none(); - } try { List changes = processWebXml(context, file); return CodemodFileScanningResult.withOnlyChanges(changes); @@ -112,4 +109,9 @@ public List getReferences() { public IncludesExcludesPattern getIncludesExcludesPattern() { return new IncludesExcludesPattern.Default(Set.of("{,**/}[wW][eE][bB].[xX][mM][lL]"), Set.of()); } + + @Override + public boolean supports(Path file) { + return "web.xml".equalsIgnoreCase(file.getFileName().toString()); + } } diff --git a/framework/codemodder-base/src/main/java/io/codemodder/CodeChanger.java b/framework/codemodder-base/src/main/java/io/codemodder/CodeChanger.java index 0d4c9a97e..db19133d5 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/CodeChanger.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/CodeChanger.java @@ -23,13 +23,20 @@ public interface CodeChanger { String getIndividualChangeDescription(final Path filePath, final CodemodChange change); /** - * A list of paths requested or rejected by the codemod. Those patterns are treated as relative to - * the repository root. + * A list of paths patterns requested or rejected by the codemod. Those patterns are treated as + * relative to the repository root. These patterns should follow the {@link + * java.nio.file.PathMatcher} specification. These patterns can be overridden by global patterns. */ default IncludesExcludesPattern getIncludesExcludesPattern() { return IncludesExcludesPattern.getAnyMatcher(); } + /** + * A predicate which dictates if the file should be inspected by the codemod. This cannot be + * overridden and should always pass before executing the codemod. + */ + boolean supports(final Path file); + /** * A lifecycle event that is called before any files are processed. This is a good place to short * circuit if you don't have the necessary resources (e.g., SARIF). diff --git a/framework/codemodder-base/src/main/java/io/codemodder/CodemodLoader.java b/framework/codemodder-base/src/main/java/io/codemodder/CodemodLoader.java index caf36f587..dbc6daf11 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/CodemodLoader.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/CodemodLoader.java @@ -108,8 +108,6 @@ public CodemodLoader( allModules.addAll(modules); } - // TODO add interested files in each provider - // record which changers are associated with which codemod ids final List codemods = new ArrayList<>(); diff --git a/framework/codemodder-base/src/main/java/io/codemodder/CompositeJavaParserChanger.java b/framework/codemodder-base/src/main/java/io/codemodder/CompositeJavaParserChanger.java index 83d38448a..cec549b7a 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/CompositeJavaParserChanger.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/CompositeJavaParserChanger.java @@ -3,6 +3,7 @@ import com.github.javaparser.ast.CompilationUnit; import io.codemodder.codetf.UnfixedFinding; import io.codemodder.javaparser.JavaParserChanger; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -35,6 +36,11 @@ public IncludesExcludesPattern getIncludesExcludesPattern() { return changers.get(0).getIncludesExcludesPattern(); } + @Override + public boolean supports(final Path file) { + return changers.stream().anyMatch(c -> c.supports(file)); + } + @Override public CodemodFileScanningResult visit( final CodemodInvocationContext context, final CompilationUnit cu) { @@ -50,9 +56,4 @@ public CodemodFileScanningResult visit( return CodemodFileScanningResult.from(changes, unfixedFindings); } - - @Override - public boolean shouldRun() { - return changers.stream().anyMatch(CodeChanger::shouldRun); - } } diff --git a/framework/codemodder-base/src/main/java/io/codemodder/LazyLoadingRuleSarif.java b/framework/codemodder-base/src/main/java/io/codemodder/LazyLoadingRuleSarif.java index 9f326ebf5..4ff624b64 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/LazyLoadingRuleSarif.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/LazyLoadingRuleSarif.java @@ -6,7 +6,6 @@ import java.nio.file.Path; import java.util.List; import java.util.Objects; -import java.util.Set; import javax.inject.Provider; /** @@ -61,11 +60,4 @@ public String getDriver() { checkInitialized(); return ruleSarif.getDriver(); } - - @Override - public Set getPaths() { - // Lazy loading sarif files are generated by running a tool before inspecting the tree - // Thus, it doesn't have a list of files beforehand and, as such, should accept any file - return Set.of("**"); - } } diff --git a/framework/codemodder-base/src/main/java/io/codemodder/RawFileCodemodRunner.java b/framework/codemodder-base/src/main/java/io/codemodder/RawFileCodemodRunner.java index e21bdfcc0..e8c817dad 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/RawFileCodemodRunner.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/RawFileCodemodRunner.java @@ -28,14 +28,11 @@ final class RawFileCodemodRunner implements CodemodRunner { @Override public boolean supports(final Path path) { - return rootedFileMatcher.shouldInspect(path.toFile()); + return rootedFileMatcher.shouldInspect(path.toFile()) && changer.supports(path); } @Override public CodemodFileScanningResult run(final CodemodInvocationContext context) throws IOException { - if (!changer.shouldRun()) { - return CodemodFileScanningResult.none(); - } return changer.visitFile(context); } } diff --git a/framework/codemodder-base/src/main/java/io/codemodder/RegexFileChanger.java b/framework/codemodder-base/src/main/java/io/codemodder/RegexFileChanger.java index 2113b0720..31f2e86bc 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/RegexFileChanger.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/RegexFileChanger.java @@ -19,17 +19,14 @@ */ public abstract class RegexFileChanger extends RawFileChanger { - private final Predicate fileMatcher; private final Pattern pattern; private final boolean removeEmptyLeftoverLines; private final List dependenciesRequired; protected RegexFileChanger( - final Predicate fileMatcher, final Pattern pattern, final boolean removeEmptyLeftoverLines, final List dependenciesRequired) { - this.fileMatcher = Objects.requireNonNull(fileMatcher, "fileMatcher"); this.pattern = Objects.requireNonNull(pattern, "pattern"); this.removeEmptyLeftoverLines = removeEmptyLeftoverLines; this.dependenciesRequired = dependenciesRequired; @@ -42,7 +39,6 @@ protected RegexFileChanger( final List dependenciesRequired, final CodemodReporterStrategy reporter) { super(reporter); - this.fileMatcher = Objects.requireNonNull(fileMatcher, "fileMatcher"); this.pattern = Objects.requireNonNull(pattern, "pattern"); this.removeEmptyLeftoverLines = removeEmptyLeftoverLines; this.dependenciesRequired = dependenciesRequired; @@ -51,10 +47,6 @@ protected RegexFileChanger( @Override public CodemodFileScanningResult visitFile(final CodemodInvocationContext context) throws IOException { - if (!fileMatcher.test(context.path())) { - return CodemodFileScanningResult.none(); - } - final List changes = new ArrayList<>(); final String fileContents = Files.readString(context.path()); final Matcher matcher = pattern.matcher(fileContents); diff --git a/framework/codemodder-base/src/main/java/io/codemodder/RuleSarif.java b/framework/codemodder-base/src/main/java/io/codemodder/RuleSarif.java index 7c43c5903..07de370ca 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/RuleSarif.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/RuleSarif.java @@ -5,7 +5,6 @@ import com.contrastsecurity.sarif.SarifSchema210; import java.nio.file.Path; import java.util.List; -import java.util.Set; /** Defines a model for interacting with SARIF. */ public interface RuleSarif { @@ -38,12 +37,6 @@ public interface RuleSarif { /** Returns the tool driver that produced this SARIF. */ String getDriver(); - /** - * Returns all file paths with results for the rule. Those paths are relative to the repository - * root. - */ - Set getPaths(); - /** An empty implementation of {@link RuleSarif} for binding codemods with no SARIF results. */ final class EmptyRuleSarif implements RuleSarif { @@ -71,10 +64,5 @@ public String getDriver() { public SarifSchema210 rawDocument() { return new SarifSchema210(); } - - @Override - public Set getPaths() { - return Set.of(); - } } } diff --git a/framework/codemodder-base/src/main/java/io/codemodder/SarifPluginJavaParserChanger.java b/framework/codemodder-base/src/main/java/io/codemodder/SarifPluginJavaParserChanger.java index 518abb689..e8f6166ff 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/SarifPluginJavaParserChanger.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/SarifPluginJavaParserChanger.java @@ -13,7 +13,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; -import java.util.Set; /** * Provides base functionality for making JavaParser-based changes based on results found by a sarif @@ -25,7 +24,6 @@ public abstract class SarifPluginJavaParserChanger extends JavaP private final Class nodeType; private final SourceCodeRegionExtractor regionExtractor; private final RegionNodeMatcher regionNodeMatcher; - private final IncludesExcludesPattern includesExcludesPattern; protected SarifPluginJavaParserChanger( final RuleSarif sarif, final Class nodeType) { @@ -79,7 +77,6 @@ protected SarifPluginJavaParserChanger( this.nodeType = Objects.requireNonNull(nodeType); this.regionExtractor = Objects.requireNonNull(regionExtractor); this.regionNodeMatcher = Objects.requireNonNull(regionNodeMatcher); - this.includesExcludesPattern = new IncludesExcludesPattern.Default(sarif.getPaths(), Set.of()); } protected SarifPluginJavaParserChanger( @@ -93,19 +90,17 @@ protected SarifPluginJavaParserChanger( this.nodeType = Objects.requireNonNull(nodeType); this.regionExtractor = Objects.requireNonNull(regionExtractor); this.regionNodeMatcher = Objects.requireNonNull(regionNodeMatcher); - this.includesExcludesPattern = new IncludesExcludesPattern.Default(sarif.getPaths(), Set.of()); + } + + @Override + public boolean supports(final Path file) { + return !sarif.getResultsByLocationPath(file).isEmpty(); } public CodemodFileScanningResult visit( final CodemodInvocationContext context, final CompilationUnit cu) { - List results = sarif.getResultsByLocationPath(context.path()); - - // small shortcut to avoid always executing the expensive findAll - if (results.isEmpty()) { - return CodemodFileScanningResult.none(); - } - List allNodes = cu.findAll(nodeType); + List results = sarif.getResultsByLocationPath(context.path()); /* * We have an interesting scenario we have to handle whereby we could accidentally feed two results that should be @@ -187,9 +182,4 @@ public boolean shouldRun() { */ public abstract ChangesResult onResultFound( CodemodInvocationContext context, CompilationUnit cu, T node, Result result); - - @Override - public IncludesExcludesPattern getIncludesExcludesPattern() { - return includesExcludesPattern; - } } diff --git a/framework/codemodder-base/src/main/java/io/codemodder/SarifPluginRawFileChanger.java b/framework/codemodder-base/src/main/java/io/codemodder/SarifPluginRawFileChanger.java index 990128b22..ff77ac2e4 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/SarifPluginRawFileChanger.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/SarifPluginRawFileChanger.java @@ -1,35 +1,32 @@ package io.codemodder; import com.contrastsecurity.sarif.Result; +import java.nio.file.Path; import java.util.List; -import java.util.Set; /** A {@link RawFileChanger} bundled with a {@link RuleSarif}. */ public abstract class SarifPluginRawFileChanger extends RawFileChanger { private final RuleSarif sarif; - private final IncludesExcludesPattern includesExcludesPattern; - protected SarifPluginRawFileChanger(final RuleSarif sarif) { this.sarif = sarif; - this.includesExcludesPattern = new IncludesExcludesPattern.Default(sarif.getPaths(), Set.of()); } protected SarifPluginRawFileChanger( final RuleSarif sarif, final CodemodReporterStrategy reporter) { super(reporter); this.sarif = sarif; - this.includesExcludesPattern = new IncludesExcludesPattern.Default(sarif.getPaths(), Set.of()); + } + + @Override + public boolean supports(final Path file) { + return !sarif.getResultsByLocationPath(file).isEmpty(); } @Override public CodemodFileScanningResult visitFile(final CodemodInvocationContext context) { - List results = sarif.getResultsByLocationPath(context.path()); - if (!results.isEmpty()) { - return onFileFound(context, results); - } - return CodemodFileScanningResult.none(); + return onFileFound(context, sarif.getResultsByLocationPath(context.path())); } /** diff --git a/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserChanger.java b/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserChanger.java index 84319abf1..892d80076 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserChanger.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserChanger.java @@ -43,6 +43,11 @@ public IncludesExcludesPattern getIncludesExcludesPattern() { return IncludesExcludesPattern.getJavaMatcher(); } + @Override + public boolean supports(final Path file) { + return file.getFileName().toString().toLowerCase().endsWith(".java"); + } + @Override public List getReferences() { return reporter.getReferences().stream().map(u -> new CodeTFReference(u, u)).toList(); diff --git a/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserCodemodRunner.java b/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserCodemodRunner.java index 0a6f1bedd..19472db5f 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserCodemodRunner.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserCodemodRunner.java @@ -43,16 +43,11 @@ public JavaParserCodemodRunner( @Override public boolean supports(final Path path) { - return path.getFileName().toString().endsWith(".java") - && rootedFileMatcher.shouldInspect(path.toFile()); + return rootedFileMatcher.shouldInspect(path.toFile()) && javaParserChanger.supports(path); } @Override public CodemodFileScanningResult run(final CodemodInvocationContext context) throws IOException { - - if (!javaParserChanger.shouldRun()) { - return CodemodFileScanningResult.none(); - } Path file = context.path(); CompilationUnit cu = parser.parseJavaFile(file); CodemodFileScanningResult result = javaParserChanger.visit(context, cu); diff --git a/framework/codemodder-base/src/test/java/io/codemodder/CLITest.java b/framework/codemodder-base/src/test/java/io/codemodder/CLITest.java index eaf1c26c9..f070904fa 100644 --- a/framework/codemodder-base/src/test/java/io/codemodder/CLITest.java +++ b/framework/codemodder-base/src/test/java/io/codemodder/CLITest.java @@ -248,6 +248,11 @@ public CodemodFileScanningResult visitFile(final CodemodInvocationContext contex public IncludesExcludesPattern getIncludesExcludesPattern() { return new IncludesExcludesPattern.Default(Set.of(), Set.of("**/*Test.java")); } + + @Override + public boolean supports(Path file) { + return true; + } } @Test diff --git a/framework/codemodder-base/src/test/java/io/codemodder/CodemodLoaderTest.java b/framework/codemodder-base/src/test/java/io/codemodder/CodemodLoaderTest.java index a0550dcdf..5f21810c2 100644 --- a/framework/codemodder-base/src/test/java/io/codemodder/CodemodLoaderTest.java +++ b/framework/codemodder-base/src/test/java/io/codemodder/CodemodLoaderTest.java @@ -54,6 +54,11 @@ public List getReferences() { public String getIndividualChangeDescription(Path filePath, CodemodChange change) { return null; } + + @Override + public boolean supports(Path file) { + return true; + } } @Test @@ -113,6 +118,11 @@ public List getReferences() { public String getIndividualChangeDescription(Path filePath, CodemodChange change) { return null; } + + @Override + public boolean supports(Path file) { + return true; + } } @Codemod( @@ -153,6 +163,11 @@ public List getReferences() { public String getIndividualChangeDescription(Path filePath, CodemodChange change) { return null; } + + @Override + public boolean supports(Path file) { + return true; + } } @Codemod( @@ -194,6 +209,11 @@ public List getReferences() { public String getIndividualChangeDescription(Path filePath, CodemodChange change) { return null; } + + @Override + public boolean supports(Path file) { + return true; + } } /** @@ -299,6 +319,11 @@ public List getReferences() { public String getIndividualChangeDescription(Path filePath, CodemodChange change) { return null; } + + @Override + public boolean supports(Path file) { + return true; + } } @Test diff --git a/plugins/codemodder-plugin-appscan/src/main/java/io/codemodder/providers/sarif/appscan/AppScanRuleSarif.java b/plugins/codemodder-plugin-appscan/src/main/java/io/codemodder/providers/sarif/appscan/AppScanRuleSarif.java index 29577bd10..a46cb928d 100644 --- a/plugins/codemodder-plugin-appscan/src/main/java/io/codemodder/providers/sarif/appscan/AppScanRuleSarif.java +++ b/plugins/codemodder-plugin-appscan/src/main/java/io/codemodder/providers/sarif/appscan/AppScanRuleSarif.java @@ -7,7 +7,6 @@ import java.io.UncheckedIOException; import java.nio.file.Path; import java.util.*; -import java.util.stream.Collectors; /** A {@link RuleSarif} for AppScan results. */ final class AppScanRuleSarif implements RuleSarif { @@ -93,11 +92,6 @@ public List getResultsByLocationPath(final Path path) { .toList()); } - @Override - public Set getPaths() { - return locations.stream().collect(Collectors.toSet()); - } - @Override public String getDriver() { return toolName; diff --git a/plugins/codemodder-plugin-appscan/src/test/java/io/codemodder/providers/sarif/appscan/AppScanModuleTest.java b/plugins/codemodder-plugin-appscan/src/test/java/io/codemodder/providers/sarif/appscan/AppScanModuleTest.java index ca76f5840..8f7fe7944 100644 --- a/plugins/codemodder-plugin-appscan/src/test/java/io/codemodder/providers/sarif/appscan/AppScanModuleTest.java +++ b/plugins/codemodder-plugin-appscan/src/test/java/io/codemodder/providers/sarif/appscan/AppScanModuleTest.java @@ -51,6 +51,11 @@ public List getReferences() { public String getIndividualChangeDescription(Path filePath, CodemodChange change) { return null; } + + @Override + public boolean supports(Path file) { + return true; + } } private static final String emptySarif = diff --git a/plugins/codemodder-plugin-codeql/src/main/java/io/codemodder/providers/sarif/codeql/CodeQLRuleSarif.java b/plugins/codemodder-plugin-codeql/src/main/java/io/codemodder/providers/sarif/codeql/CodeQLRuleSarif.java index 203cacb1a..a78734538 100644 --- a/plugins/codemodder-plugin-codeql/src/main/java/io/codemodder/providers/sarif/codeql/CodeQLRuleSarif.java +++ b/plugins/codemodder-plugin-codeql/src/main/java/io/codemodder/providers/sarif/codeql/CodeQLRuleSarif.java @@ -21,7 +21,6 @@ public final class CodeQLRuleSarif implements RuleSarif { private final String ruleId; private final Map> resultsCache; private final Path repositoryRoot; - private Set relativeLocations; public CodeQLRuleSarif( final String ruleId, final SarifSchema210 sarif, final CodeDirectory codeDirectory) { @@ -29,7 +28,6 @@ public CodeQLRuleSarif( this.ruleId = Objects.requireNonNull(ruleId); this.repositoryRoot = codeDirectory.asPath(); this.resultsCache = new HashMap<>(); - this.relativeLocations = null; } private String extractRuleId(final Result result, final Run run) { @@ -95,28 +93,6 @@ public List getResultsByLocationPath(final Path path) { return results; } - @Override - public Set getPaths() { - if (relativeLocations == null) { - relativeLocations = - sarif.getRuns().stream() - .flatMap( - run -> - run.getResults().stream() - .filter(result -> ruleId.equals(extractRuleId(result, run)))) - .map( - result -> - result - .getLocations() - .get(0) - .getPhysicalLocation() - .getArtifactLocation() - .getUri()) - .collect(Collectors.toSet()); - } - return relativeLocations; - } - @Override public SarifSchema210 rawDocument() { return sarif; diff --git a/plugins/codemodder-plugin-pmd/src/main/java/io/codemodder/providers/sarif/pmd/PmdRuleSarif.java b/plugins/codemodder-plugin-pmd/src/main/java/io/codemodder/providers/sarif/pmd/PmdRuleSarif.java index c2058f0f6..27d41968b 100644 --- a/plugins/codemodder-plugin-pmd/src/main/java/io/codemodder/providers/sarif/pmd/PmdRuleSarif.java +++ b/plugins/codemodder-plugin-pmd/src/main/java/io/codemodder/providers/sarif/pmd/PmdRuleSarif.java @@ -8,8 +8,6 @@ import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Set; -import java.util.stream.Collectors; final class PmdRuleSarif implements RuleSarif { @@ -17,7 +15,6 @@ final class PmdRuleSarif implements RuleSarif { private final String ruleId; private final Map> resultsByFile; private final Path repositoryRoot; - private Set relativeLocations; PmdRuleSarif( final String ruleId, @@ -28,7 +25,6 @@ final class PmdRuleSarif implements RuleSarif { this.sarif = Objects.requireNonNull(sarif); this.resultsByFile = Objects.requireNonNull(resultsByFile); this.repositoryRoot = repositoryRoot; - this.relativeLocations = null; } @Override @@ -60,15 +56,4 @@ public String getDriver() { } static final String toolName = "pmd"; - - @Override - public Set getPaths() { - if (this.relativeLocations == null) { - relativeLocations = - resultsByFile.keySet().stream() - .map(s -> repositoryRoot.relativize(Path.of(s)).toString()) - .collect(Collectors.toSet()); - } - return relativeLocations; - } } diff --git a/plugins/codemodder-plugin-semgrep/src/main/java/io/codemodder/providers/sarif/semgrep/SingleSemgrepRuleSarif.java b/plugins/codemodder-plugin-semgrep/src/main/java/io/codemodder/providers/sarif/semgrep/SingleSemgrepRuleSarif.java index 601c0255e..2dc463f4f 100644 --- a/plugins/codemodder-plugin-semgrep/src/main/java/io/codemodder/providers/sarif/semgrep/SingleSemgrepRuleSarif.java +++ b/plugins/codemodder-plugin-semgrep/src/main/java/io/codemodder/providers/sarif/semgrep/SingleSemgrepRuleSarif.java @@ -8,7 +8,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.*; -import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -25,7 +24,6 @@ final class SingleSemgrepRuleSarif implements RuleSarif { private final String ruleId; private final Map> resultsCache; private final Path repositoryRoot; - private Set relativeLocations; SingleSemgrepRuleSarif( final String ruleId, final SarifSchema210 sarif, final Path codeDirectory) { @@ -33,7 +31,6 @@ final class SingleSemgrepRuleSarif implements RuleSarif { this.ruleId = Objects.requireNonNull(ruleId); this.repositoryRoot = Objects.requireNonNull(codeDirectory); this.resultsCache = new HashMap<>(); - this.relativeLocations = null; } @Override @@ -92,38 +89,6 @@ public List getResultsByLocationPath(final Path path) { return results; } - @Override - public Set getPaths() { - if (relativeLocations == null) { - relativeLocations = - sarif.getRuns().get(0).getResults().stream() - /* - * The default Semgrep rules have a rule id reported that is what you'd expect. When you run - * your own custom rules locally, they'll contain part of the file system path to the rule. - * - * Because this provides support for both types, we need this check to account for which type - * of rule id we're dealing with. - */ - .filter( - result -> - result.getRuleId().endsWith("." + ruleId) - || result.getRuleId().equals(ruleId)) - .map( - result -> - result - .getLocations() - .get(0) - .getPhysicalLocation() - .getArtifactLocation() - .getUri()) - .map(s -> Path.of(s)) - .map(p -> p.startsWith(repositoryRoot) ? repositoryRoot.relativize(p) : p) - .map(p -> p.toString()) - .collect(Collectors.toSet()); - } - return relativeLocations; - } - @Override public String getDriver() { return sarif.getRuns().get(0).getTool().getDriver().getName(); diff --git a/plugins/codemodder-plugin-semgrep/src/test/java/io/codemodder/providers/sarif/semgrep/SemgrepModuleTest.java b/plugins/codemodder-plugin-semgrep/src/test/java/io/codemodder/providers/sarif/semgrep/SemgrepModuleTest.java index f5bd8d469..0cb1b5b82 100644 --- a/plugins/codemodder-plugin-semgrep/src/test/java/io/codemodder/providers/sarif/semgrep/SemgrepModuleTest.java +++ b/plugins/codemodder-plugin-semgrep/src/test/java/io/codemodder/providers/sarif/semgrep/SemgrepModuleTest.java @@ -68,6 +68,11 @@ public List getReferences() { public String getIndividualChangeDescription(Path filePath, CodemodChange change) { return null; } + + @Override + public boolean supports(Path file) { + return true; + } } @Codemod( @@ -218,7 +223,7 @@ void it_detects_rule_ids() { } private SemgrepModule createModule( - final Path dir, final List> codemodTypes) throws IOException { + final Path dir, final List> codemodTypes) { return new SemgrepModule(dir, List.of("**"), List.of(), codemodTypes); } @@ -301,6 +306,11 @@ public List getReferences() { public String getIndividualChangeDescription(Path filePath, CodemodChange change) { return null; } + + @Override + public boolean supports(final Path file) { + return true; + } } private static final String vulnCode = diff --git a/plugins/codemodder-plugin-semgrep/src/test/java/io/codemodder/providers/sarif/semgrep/invalid/bindstoincorrect/BindsToIncorrectObject.java b/plugins/codemodder-plugin-semgrep/src/test/java/io/codemodder/providers/sarif/semgrep/invalid/bindstoincorrect/BindsToIncorrectObject.java index 5f7581216..81c9d442b 100644 --- a/plugins/codemodder-plugin-semgrep/src/test/java/io/codemodder/providers/sarif/semgrep/invalid/bindstoincorrect/BindsToIncorrectObject.java +++ b/plugins/codemodder-plugin-semgrep/src/test/java/io/codemodder/providers/sarif/semgrep/invalid/bindstoincorrect/BindsToIncorrectObject.java @@ -41,4 +41,9 @@ public List getReferences() { public String getIndividualChangeDescription(Path filePath, CodemodChange change) { return null; } + + @Override + public boolean supports(final Path file) { + return true; + } } diff --git a/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/DefaultRuleFinding.java b/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/DefaultRuleFinding.java index 149b1f8d4..cd219e586 100644 --- a/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/DefaultRuleFinding.java +++ b/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/DefaultRuleFinding.java @@ -3,8 +3,6 @@ import io.codemodder.sonar.model.SonarFinding; import java.nio.file.Path; import java.util.*; -import java.util.stream.Collectors; -import java.util.stream.Stream; abstract class DefaultRuleFinding implements RuleFinding { @@ -34,11 +32,4 @@ public List getResultsByPath(final Path path) { public boolean hasResults() { return !findings.isEmpty(); } - - @Override - public Set getPaths() { - var pathSet = new HashSet(); - Stream allFindings = findings.values().stream().flatMap(l -> l.stream()); - return allFindings.flatMap(f -> f.componentFileName().stream()).collect(Collectors.toSet()); - } } diff --git a/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/RuleFinding.java b/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/RuleFinding.java index 553946591..cfb01ab01 100644 --- a/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/RuleFinding.java +++ b/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/RuleFinding.java @@ -3,7 +3,6 @@ import io.codemodder.sonar.model.SonarFinding; import java.nio.file.Path; import java.util.List; -import java.util.Set; /** A view of the Sonar findings results file for a given rule. */ public interface RuleFinding { @@ -13,7 +12,4 @@ public interface RuleFinding { /** Whether any results are available. */ boolean hasResults(); - - /** Get all file paths that have any results. */ - Set getPaths(); } diff --git a/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/SonarPluginJavaParserChanger.java b/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/SonarPluginJavaParserChanger.java index 7e8cef2df..1b10fd5d5 100644 --- a/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/SonarPluginJavaParserChanger.java +++ b/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/SonarPluginJavaParserChanger.java @@ -8,10 +8,10 @@ import io.codemodder.javaparser.ChangesResult; import io.codemodder.javaparser.JavaParserChanger; import io.codemodder.sonar.model.SonarFinding; +import java.nio.file.Path; import java.util.ArrayList; import java.util.List; import java.util.Objects; -import java.util.Set; /** Provides base functionality for making JavaParser-based changes based on Sonar results. */ public abstract class SonarPluginJavaParserChanger @@ -23,8 +23,6 @@ public abstract class SonarPluginJavaParserChanger ruleFinding, final Class nodeType, @@ -34,8 +32,6 @@ protected SonarPluginJavaParserChanger( this.nodeType = Objects.requireNonNull(nodeType); this.regionNodeMatcher = regionNodeMatcher; this.nodeCollector = nodeCollector; - this.includesExcludesPattern = - new IncludesExcludesPattern.Default(ruleFinding.getPaths(), Set.of()); } protected SonarPluginJavaParserChanger( @@ -53,20 +49,19 @@ protected SonarPluginJavaParserChanger( this.nodeType = Objects.requireNonNull(nodeType); this.regionNodeMatcher = regionNodeMatcher; this.nodeCollector = NodeCollector.ALL_FROM_TYPE; - this.includesExcludesPattern = - new IncludesExcludesPattern.Default(ruleFinding.getPaths(), Set.of()); + } + + @Override + public boolean supports(final Path file) { + List findings = ruleFinding.getResultsByPath(file); + return super.supports(file) && findings != null && !findings.isEmpty(); } @Override public CodemodFileScanningResult visit( final CodemodInvocationContext context, final CompilationUnit cu) { - List findings = ruleFinding.getResultsByPath(context.path()); - - // small shortcut to avoid always executing the expensive findAll - if (findings == null || findings.isEmpty()) { - return CodemodFileScanningResult.none(); - } final List allNodes = nodeCollector.collectNodes(cu, nodeType); + List findings = ruleFinding.getResultsByPath(context.path()); List codemodChanges = new ArrayList<>(); for (SonarFinding sonarFinding : findings) { @@ -105,11 +100,6 @@ public CodemodFileScanningResult visit( return CodemodFileScanningResult.withOnlyChanges(codemodChanges); } - @Override - public boolean shouldRun() { - return ruleFinding.hasResults(); - } - /** * Creates a visitor for the given context and locations. * diff --git a/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/SonarRemediatingJavaParserChanger.java b/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/SonarRemediatingJavaParserChanger.java index a9980e83c..6d76195fd 100644 --- a/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/SonarRemediatingJavaParserChanger.java +++ b/plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/SonarRemediatingJavaParserChanger.java @@ -2,37 +2,28 @@ import io.codemodder.CodemodReporterStrategy; import io.codemodder.FixOnlyCodeChanger; -import io.codemodder.IncludesExcludesPattern; import io.codemodder.javaparser.JavaParserChanger; -import java.util.Set; +import java.nio.file.Path; /** Provides base functionality for making JavaParser-based remediation of Sonar results. */ public abstract class SonarRemediatingJavaParserChanger extends JavaParserChanger implements FixOnlyCodeChanger { private final boolean shouldRun; - private final IncludesExcludesPattern includesExcludesPattern; protected SonarRemediatingJavaParserChanger( final CodemodReporterStrategy reporter, final RuleFinding findings) { super(reporter); this.shouldRun = findings.hasResults(); - Set allPathFindings = findings.getPaths(); - this.includesExcludesPattern = new IncludesExcludesPattern.Default(allPathFindings, Set.of()); } @Override - public IncludesExcludesPattern getIncludesExcludesPattern() { - return this.includesExcludesPattern; + public boolean supports(final Path file) { + return super.supports(file) && this.shouldRun; } @Override public String vendorName() { return "Sonar"; } - - @Override - public boolean shouldRun() { - return shouldRun; - } } From 0df51f2be01bafef9d83142fae9fb64bc02b05e3 Mon Sep 17 00:00:00 2001 From: andrecs <12188364+andrecsilva@users.noreply.github.com> Date: Fri, 16 Aug 2024 08:57:56 -0300 Subject: [PATCH 5/8] Tests for includes/excludes override and supports check --- .../src/test/java/io/codemodder/CLITest.java | 34 ++++++++++++++----- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/framework/codemodder-base/src/test/java/io/codemodder/CLITest.java b/framework/codemodder-base/src/test/java/io/codemodder/CLITest.java index f070904fa..ee83b0d08 100644 --- a/framework/codemodder-base/src/test/java/io/codemodder/CLITest.java +++ b/framework/codemodder-base/src/test/java/io/codemodder/CLITest.java @@ -37,6 +37,7 @@ final class CLITest { private Path fooJavaFile; private Path barJavaFile; private Path testFile; + private Path notJavaFile; private List sourceDirectories; @BeforeEach @@ -49,11 +50,13 @@ void setup(final @TempDir Path tmpDir) throws IOException { fooJavaFile = module1JavaDir.resolve("Foo.java"); barJavaFile = module2JavaDir.resolve("Bar.java"); testFile = module2JavaDir.resolve("MyTest.java"); + notJavaFile = module2JavaDir.resolve("MyTest.txt"); Files.write( fooJavaFile, "import com.acme.util.Bar; class Foo {private var bar = new Bar();}".getBytes()); Files.write(barJavaFile, "public class Bar {}".getBytes()); Files.write(testFile, "public class MyTest {}".getBytes()); + Files.write(notJavaFile, "ground0".getBytes()); /* * Only add module2 to the discovered source directories. This will help prove that the module1 files can still be seen and changed, even if we couldn't locate it as a "source directory". @@ -82,6 +85,22 @@ void it_doesnt_change_test_file() throws IOException { assertThat(Files.readString(testFile)).doesNotContain("cloud9"); } + @Test + void global_overrides_codemod_includes_excludes() throws IOException { + String[] args = + new String[] {"--dont-exit", "--path-include=**/*Test.java", workingRepoDir.toString()}; + Runner.run(List.of(Cloud9Changer.class), args); + assertThat(Files.readString(testFile)).contains("cloud9"); + } + + @Test + void it_must_respect_codemod_supports() throws IOException { + String[] args = + new String[] {"--dont-exit", "--path-include=**/*Test.txt", workingRepoDir.toString()}; + Runner.run(List.of(Cloud9Changer.class), args); + assertThat(Files.readString(notJavaFile)).doesNotContain("cloud9"); + } + @Test void it_works_without_output_file() throws IOException { String[] args = new String[] {"--dont-exit", workingRepoDir.toString()}; @@ -214,7 +233,7 @@ void file_finder_works() throws IOException { IncludesExcludes all = IncludesExcludes.any(); List files = finder.findFiles(workingRepoDir, all); - assertThat(files).containsExactly(fooJavaFile, barJavaFile, testFile); + assertThat(files).containsExactly(fooJavaFile, barJavaFile, testFile, notJavaFile); IncludesExcludes onlyFoo = IncludesExcludes.withSettings(workingRepoDir.toFile(), List.of("**/Foo.java"), List.of()); @@ -236,12 +255,9 @@ private Cloud9Changer() { public CodemodFileScanningResult visitFile(final CodemodInvocationContext context) throws IOException { Path path = context.path(); - if (path.toString().endsWith(".java")) { - Files.writeString(path, "cloud9"); - List changes = List.of(CodemodChange.from(1)); - return CodemodFileScanningResult.withOnlyChanges(changes); - } - return CodemodFileScanningResult.none(); + Files.writeString(path, "cloud9"); + List changes = List.of(CodemodChange.from(1)); + return CodemodFileScanningResult.withOnlyChanges(changes); } @Override @@ -250,8 +266,8 @@ public IncludesExcludesPattern getIncludesExcludesPattern() { } @Override - public boolean supports(Path file) { - return true; + public boolean supports(final Path file) { + return file.toString().endsWith(".java"); } } From f2c89e45ac5a02aa9b186c28e2163cfe52cf67ec Mon Sep 17 00:00:00 2001 From: andrecs <12188364+andrecsilva@users.noreply.github.com> Date: Fri, 16 Aug 2024 09:36:01 -0300 Subject: [PATCH 6/8] Linting and documentation --- .../codemodder/IncludesExcludesPattern.java | 19 ++++++++++++++----- .../java/io/codemodder/RegexFileChanger.java | 3 --- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/framework/codemodder-base/src/main/java/io/codemodder/IncludesExcludesPattern.java b/framework/codemodder-base/src/main/java/io/codemodder/IncludesExcludesPattern.java index a7f8c8db0..00b802b78 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/IncludesExcludesPattern.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/IncludesExcludesPattern.java @@ -7,10 +7,11 @@ public interface IncludesExcludesPattern { /** - * Returns an IncludesExcludes file matcher that matches files following the patterns against a - * root folder. + * Returns an IncludesExcludes file matcher that matches files following the patterns specified + * patterns. The patterns must follow the java's {@link java.nio.file.PathMatcher} specification + * and are treated as relative paths with regard to the repository root. */ - public IncludesExcludes getRootedMatcher(final Path root); + IncludesExcludes getRootedMatcher(final Path root); class Default implements IncludesExcludesPattern { @@ -34,9 +35,12 @@ public IncludesExcludes getRootedMatcher(final Path root) { } } + /** An includes/excludes pattern that matches java files. */ final class JavaMatcherSingleton { private static IncludesExcludesPattern singleton; + private JavaMatcherSingleton() {} + public static IncludesExcludesPattern getInstance() { if (singleton == null) { singleton = @@ -47,9 +51,12 @@ public static IncludesExcludesPattern getInstance() { } } + /** An includes/excludes pattern that matches any files. */ final class AnySingleton { private static IncludesExcludesPattern singleton; + private AnySingleton() {} + public static IncludesExcludesPattern getInstance() { if (singleton == null) { singleton = new IncludesExcludesPattern.Default(Set.of("**"), Collections.emptySet()); @@ -58,11 +65,13 @@ public static IncludesExcludesPattern getInstance() { } } - public static IncludesExcludesPattern getJavaMatcher() { + /** Returns an includes/excludes pattern that matches any java files. */ + static IncludesExcludesPattern getJavaMatcher() { return JavaMatcherSingleton.getInstance(); } - public static IncludesExcludesPattern getAnyMatcher() { + /** Returns an includes/excludes pattern that matches any files. */ + static IncludesExcludesPattern getAnyMatcher() { return AnySingleton.getInstance(); } } diff --git a/framework/codemodder-base/src/main/java/io/codemodder/RegexFileChanger.java b/framework/codemodder-base/src/main/java/io/codemodder/RegexFileChanger.java index 31f2e86bc..092627b04 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/RegexFileChanger.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/RegexFileChanger.java @@ -4,11 +4,9 @@ import java.io.LineNumberReader; import java.io.StringReader; import java.nio.file.Files; -import java.nio.file.Path; import java.util.ArrayList; import java.util.List; import java.util.Objects; -import java.util.function.Predicate; import java.util.regex.Matcher; import java.util.regex.Pattern; import org.apache.commons.lang3.StringUtils; @@ -33,7 +31,6 @@ protected RegexFileChanger( } protected RegexFileChanger( - final Predicate fileMatcher, final Pattern pattern, final boolean removeEmptyLeftoverLines, final List dependenciesRequired, From ab03c2743c5f860b0e8902abbff27f38d9a68b6e Mon Sep 17 00:00:00 2001 From: andrecs <12188364+andrecsilva@users.noreply.github.com> Date: Mon, 19 Aug 2024 07:27:37 -0300 Subject: [PATCH 7/8] Requested changes --- .../io/codemodder/codemods/JSPScriptletXSSCodemod.java | 2 +- .../java/io/codemodder/codemods/VerbTamperingCodemod.java | 2 +- .../main/java/io/codemodder/IncludesExcludesPattern.java | 8 ++++---- .../src/test/java/io/codemodder/CodemodLoaderTest.java | 6 +++--- .../providers/sarif/appscan/AppScanModuleTest.java | 2 +- .../providers/sarif/semgrep/SemgrepModuleTest.java | 2 +- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/core-codemods/src/main/java/io/codemodder/codemods/JSPScriptletXSSCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/JSPScriptletXSSCodemod.java index 7c286cbfd..7b9272a6d 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/JSPScriptletXSSCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/JSPScriptletXSSCodemod.java @@ -37,7 +37,7 @@ public String getReplacementFor(final String matchingSnippet) { } @Override - public boolean supports(Path file) { + public boolean supports(final Path file) { return file.getFileName().toString().toLowerCase().endsWith(".jsp"); } diff --git a/core-codemods/src/main/java/io/codemodder/codemods/VerbTamperingCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/VerbTamperingCodemod.java index 340dc2c9c..bfa9ddd69 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/VerbTamperingCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/VerbTamperingCodemod.java @@ -111,7 +111,7 @@ public IncludesExcludesPattern getIncludesExcludesPattern() { } @Override - public boolean supports(Path file) { + public boolean supports(final Path file) { return "web.xml".equalsIgnoreCase(file.getFileName().toString()); } } diff --git a/framework/codemodder-base/src/main/java/io/codemodder/IncludesExcludesPattern.java b/framework/codemodder-base/src/main/java/io/codemodder/IncludesExcludesPattern.java index 00b802b78..004f0b882 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/IncludesExcludesPattern.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/IncludesExcludesPattern.java @@ -19,8 +19,8 @@ class Default implements IncludesExcludesPattern { private final Set pathExcludes; public Default(final Set pathIncludes, final Set pathExcludes) { - this.pathIncludes = pathIncludes; - this.pathExcludes = pathExcludes; + this.pathIncludes = Objects.requireNonNull(pathIncludes); + this.pathExcludes = Objects.requireNonNull(pathExcludes); } @Override @@ -41,7 +41,7 @@ final class JavaMatcherSingleton { private JavaMatcherSingleton() {} - public static IncludesExcludesPattern getInstance() { + static IncludesExcludesPattern getInstance() { if (singleton == null) { singleton = new IncludesExcludesPattern.Default( @@ -57,7 +57,7 @@ final class AnySingleton { private AnySingleton() {} - public static IncludesExcludesPattern getInstance() { + static IncludesExcludesPattern getInstance() { if (singleton == null) { singleton = new IncludesExcludesPattern.Default(Set.of("**"), Collections.emptySet()); } diff --git a/framework/codemodder-base/src/test/java/io/codemodder/CodemodLoaderTest.java b/framework/codemodder-base/src/test/java/io/codemodder/CodemodLoaderTest.java index 5f21810c2..cea92f74c 100644 --- a/framework/codemodder-base/src/test/java/io/codemodder/CodemodLoaderTest.java +++ b/framework/codemodder-base/src/test/java/io/codemodder/CodemodLoaderTest.java @@ -56,7 +56,7 @@ public String getIndividualChangeDescription(Path filePath, CodemodChange change } @Override - public boolean supports(Path file) { + public boolean supports(final Path file) { return true; } } @@ -120,7 +120,7 @@ public String getIndividualChangeDescription(Path filePath, CodemodChange change } @Override - public boolean supports(Path file) { + public boolean supports(final Path file) { return true; } } @@ -321,7 +321,7 @@ public String getIndividualChangeDescription(Path filePath, CodemodChange change } @Override - public boolean supports(Path file) { + public boolean supports(final Path file) { return true; } } diff --git a/plugins/codemodder-plugin-appscan/src/test/java/io/codemodder/providers/sarif/appscan/AppScanModuleTest.java b/plugins/codemodder-plugin-appscan/src/test/java/io/codemodder/providers/sarif/appscan/AppScanModuleTest.java index 8f7fe7944..90a92285b 100644 --- a/plugins/codemodder-plugin-appscan/src/test/java/io/codemodder/providers/sarif/appscan/AppScanModuleTest.java +++ b/plugins/codemodder-plugin-appscan/src/test/java/io/codemodder/providers/sarif/appscan/AppScanModuleTest.java @@ -53,7 +53,7 @@ public String getIndividualChangeDescription(Path filePath, CodemodChange change } @Override - public boolean supports(Path file) { + public boolean supports(final Path file) { return true; } } diff --git a/plugins/codemodder-plugin-semgrep/src/test/java/io/codemodder/providers/sarif/semgrep/SemgrepModuleTest.java b/plugins/codemodder-plugin-semgrep/src/test/java/io/codemodder/providers/sarif/semgrep/SemgrepModuleTest.java index 0cb1b5b82..bde1739ca 100644 --- a/plugins/codemodder-plugin-semgrep/src/test/java/io/codemodder/providers/sarif/semgrep/SemgrepModuleTest.java +++ b/plugins/codemodder-plugin-semgrep/src/test/java/io/codemodder/providers/sarif/semgrep/SemgrepModuleTest.java @@ -70,7 +70,7 @@ public String getIndividualChangeDescription(Path filePath, CodemodChange change } @Override - public boolean supports(Path file) { + public boolean supports(final Path file) { return true; } } From df5de2b750d39034aa8b700cd5d38f62e2cde144 Mon Sep 17 00:00:00 2001 From: andrecs <12188364+andrecsilva@users.noreply.github.com> Date: Fri, 23 Aug 2024 07:35:56 -0300 Subject: [PATCH 8/8] Refactored includes/excludes default check --- .../src/main/java/io/codemodder/CLI.java | 43 ++++++++++----- .../io/codemodder/DefaultCodemodExecutor.java | 54 +++++++++++++++---- .../io/codemodder/RawFileCodemodRunner.java | 15 +++--- .../javaparser/JavaParserCodemodRunner.java | 19 ++++--- 4 files changed, 94 insertions(+), 37 deletions(-) diff --git a/framework/codemodder-base/src/main/java/io/codemodder/CLI.java b/framework/codemodder-base/src/main/java/io/codemodder/CLI.java index e9d5daecc..f0297d0dd 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/CLI.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/CLI.java @@ -347,7 +347,6 @@ public Integer call() throws IOException { List pathExcludes = this.pathExcludes; // If no path includes/excludes were specified, relegate filtering to each codemod - // This is indicated by the MatchesEverything type of IncludesExcludes boolean useGlobalIncludesExcludes = pathIncludes != null || pathExcludes != null; IncludesExcludes includesExcludes = IncludesExcludes.any(); if (pathIncludes == null) { @@ -439,19 +438,35 @@ public Integer call() throws IOException { FileCache fileCache = FileCache.createDefault(maxFileCacheSize); for (CodemodIdPair codemod : codemods) { - CodemodExecutor codemodExecutor = - new DefaultCodemodExecutor( - projectPath, - includesExcludes, - codemod, - projectProviders, - codeTFProviders, - fileCache, - javaParserFacade, - encodingDetector, - maxFileSize, - maxFiles, - maxWorkers); + CodemodExecutor codemodExecutor; + if (useGlobalIncludesExcludes) { + codemodExecutor = + new DefaultCodemodExecutor( + projectPath, + includesExcludes, + codemod, + projectProviders, + codeTFProviders, + fileCache, + javaParserFacade, + encodingDetector, + maxFileSize, + maxFiles, + maxWorkers); + } else { + codemodExecutor = + new DefaultCodemodExecutor( + projectPath, + codemod, + projectProviders, + codeTFProviders, + fileCache, + javaParserFacade, + encodingDetector, + maxFileSize, + maxFiles, + maxWorkers); + } log.info("running codemod: {}", codemod.getId()); CodeTFResult result = codemodExecutor.execute(filePaths); diff --git a/framework/codemodder-base/src/main/java/io/codemodder/DefaultCodemodExecutor.java b/framework/codemodder-base/src/main/java/io/codemodder/DefaultCodemodExecutor.java index 3aab72338..75c3afed2 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/DefaultCodemodExecutor.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/DefaultCodemodExecutor.java @@ -30,6 +30,7 @@ final class DefaultCodemodExecutor implements CodemodExecutor { private final IncludesExcludes includesExcludes; private final EncodingDetector encodingDetector; private final FileCache fileCache; + private final boolean perCodemodIncludesExcludes; /** The max size of a file we'll scan. If a file is larger than this, we'll skip it. */ private final int maxFileSize; @@ -43,6 +44,31 @@ final class DefaultCodemodExecutor implements CodemodExecutor { /** The max number of workers we'll use to scan files. */ private final int maxWorkers; + DefaultCodemodExecutor( + final Path projectDir, + final CodemodIdPair codemod, + final List projectProviders, + final List codetfProviders, + final FileCache fileCache, + final JavaParserFacade javaParserFacade, + final EncodingDetector encodingDetector, + final int maxFileSize, + final int maxFiles, + final int maxWorkers) { + this.projectDir = Objects.requireNonNull(projectDir); + this.includesExcludes = IncludesExcludes.any(); + this.perCodemodIncludesExcludes = true; + this.codemod = Objects.requireNonNull(codemod); + this.codetfProviders = Objects.requireNonNull(codetfProviders); + this.projectProviders = Objects.requireNonNull(projectProviders); + this.javaParserFacade = Objects.requireNonNull(javaParserFacade); + this.fileCache = Objects.requireNonNull(fileCache); + this.encodingDetector = Objects.requireNonNull(encodingDetector); + this.maxFileSize = maxFileSize; + this.maxFiles = maxFiles; + this.maxWorkers = maxWorkers; + } + DefaultCodemodExecutor( final Path projectDir, final IncludesExcludes includesExcludes, @@ -57,6 +83,7 @@ final class DefaultCodemodExecutor implements CodemodExecutor { final int maxWorkers) { this.projectDir = Objects.requireNonNull(projectDir); this.includesExcludes = Objects.requireNonNull(includesExcludes); + this.perCodemodIncludesExcludes = false; this.codemod = Objects.requireNonNull(codemod); this.codetfProviders = Objects.requireNonNull(codetfProviders); this.projectProviders = Objects.requireNonNull(projectProviders); @@ -80,16 +107,25 @@ public CodeTFResult execute(final List filePaths) { */ CodemodRunner codemodRunner; if (codeChanger instanceof JavaParserChanger) { - codemodRunner = - new JavaParserCodemodRunner( - javaParserFacade, - (JavaParserChanger) codeChanger, - projectDir, - includesExcludes, - encodingDetector); + if (perCodemodIncludesExcludes) { + codemodRunner = + new JavaParserCodemodRunner( + javaParserFacade, (JavaParserChanger) codeChanger, projectDir, encodingDetector); + } else { + codemodRunner = + new JavaParserCodemodRunner( + javaParserFacade, + (JavaParserChanger) codeChanger, + projectDir, + includesExcludes, + encodingDetector); + } } else if (codeChanger instanceof RawFileChanger) { - codemodRunner = - new RawFileCodemodRunner((RawFileChanger) codeChanger, projectDir, includesExcludes); + if (perCodemodIncludesExcludes) { + codemodRunner = new RawFileCodemodRunner((RawFileChanger) codeChanger, projectDir); + } else { + codemodRunner = new RawFileCodemodRunner((RawFileChanger) codeChanger, includesExcludes); + } } else { throw new UnsupportedOperationException( "unsupported codeChanger type: " + codeChanger.getClass().getName()); diff --git a/framework/codemodder-base/src/main/java/io/codemodder/RawFileCodemodRunner.java b/framework/codemodder-base/src/main/java/io/codemodder/RawFileCodemodRunner.java index e8c817dad..bd94dabca 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/RawFileCodemodRunner.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/RawFileCodemodRunner.java @@ -14,16 +14,15 @@ final class RawFileCodemodRunner implements CodemodRunner { private final RawFileChanger changer; private final IncludesExcludes rootedFileMatcher; + RawFileCodemodRunner(final RawFileChanger changer, final Path projectDir) { + this.changer = Objects.requireNonNull(changer); + this.rootedFileMatcher = changer.getIncludesExcludesPattern().getRootedMatcher(projectDir); + } + RawFileCodemodRunner( - final RawFileChanger changer, - final Path projectDir, - final IncludesExcludes globalIncludesExcludes) { + final RawFileChanger changer, final IncludesExcludes globalIncludesExcludes) { this.changer = Objects.requireNonNull(changer); - if (globalIncludesExcludes instanceof IncludesExcludes.MatchesEverything) { - this.rootedFileMatcher = changer.getIncludesExcludesPattern().getRootedMatcher(projectDir); - } else { - this.rootedFileMatcher = Objects.requireNonNull(globalIncludesExcludes); - } + this.rootedFileMatcher = Objects.requireNonNull(globalIncludesExcludes); } @Override diff --git a/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserCodemodRunner.java b/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserCodemodRunner.java index 19472db5f..82138c715 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserCodemodRunner.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/javaparser/JavaParserCodemodRunner.java @@ -32,13 +32,20 @@ public JavaParserCodemodRunner( this.parser = Objects.requireNonNull(parser); this.javaParserChanger = Objects.requireNonNull(javaParserChanger); this.encodingDetector = Objects.requireNonNull(encodingDetector); + this.rootedFileMatcher = Objects.requireNonNull(globalIncludesExcludes); + } + + public JavaParserCodemodRunner( + final JavaParserFacade parser, + final JavaParserChanger javaParserChanger, + final Path projectDir, + final EncodingDetector encodingDetector) { + this.parser = Objects.requireNonNull(parser); + this.javaParserChanger = Objects.requireNonNull(javaParserChanger); + this.encodingDetector = Objects.requireNonNull(encodingDetector); // If no global includes/excludes are specified, each codemod will decide which files to inspect - if (globalIncludesExcludes instanceof IncludesExcludes.MatchesEverything) { - this.rootedFileMatcher = - javaParserChanger.getIncludesExcludesPattern().getRootedMatcher(projectDir); - } else { - this.rootedFileMatcher = Objects.requireNonNull(globalIncludesExcludes); - } + this.rootedFileMatcher = + javaParserChanger.getIncludesExcludesPattern().getRootedMatcher(projectDir); } @Override