diff --git a/core-codemods/src/main/java/io/codemodder/codemods/AddMissingOverrideCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/AddMissingOverrideCodemod.java index 65737237f..5c59f1716 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/AddMissingOverrideCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/AddMissingOverrideCodemod.java @@ -29,7 +29,7 @@ public AddMissingOverrideCodemod( } @Override - protected DetectorRule getDetectorRule() { + public DetectorRule getDetectorRule() { return new DetectorRule( "java:S1161", "`@Override` should be used on overriding and implementing methods", diff --git a/core-codemods/src/main/java/io/codemodder/codemods/AvoidImplicitPublicConstructorCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/AvoidImplicitPublicConstructorCodemod.java index 1ee5bb140..e2cead5b7 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/AvoidImplicitPublicConstructorCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/AvoidImplicitPublicConstructorCodemod.java @@ -33,7 +33,8 @@ public AvoidImplicitPublicConstructorCodemod( super(issues, SimpleName.class); } - protected DetectorRule getDetectorRule() { + @Override + public DetectorRule getDetectorRule() { return new DetectorRule( "java:S1118", "Utility classes should not have public constructors", diff --git a/core-codemods/src/main/java/io/codemodder/codemods/DeclareVariableOnSeparateLineCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/DeclareVariableOnSeparateLineCodemod.java index 2656483c4..4c9df403c 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/DeclareVariableOnSeparateLineCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/DeclareVariableOnSeparateLineCodemod.java @@ -31,7 +31,7 @@ public DeclareVariableOnSeparateLineCodemod( } @Override - protected DetectorRule getDetectorRule() { + public DetectorRule getDetectorRule() { return new DetectorRule( "java:S1659", "Multiple variables should not be declared on the same line", diff --git a/core-codemods/src/main/java/io/codemodder/codemods/DefectDojoSqlInjectionCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/DefectDojoSqlInjectionCodemod.java index bb053ca7a..15ca9a719 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/DefectDojoSqlInjectionCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/DefectDojoSqlInjectionCodemod.java @@ -3,9 +3,9 @@ import com.github.javaparser.ast.CompilationUnit; import com.github.javaparser.ast.expr.MethodCallExpr; import io.codemodder.*; -import io.codemodder.codetf.DetectionTool; -import io.codemodder.codetf.DetectorFinding; import io.codemodder.codetf.DetectorRule; +import io.codemodder.codetf.FixedFinding; +import io.codemodder.codetf.UnfixedFinding; import io.codemodder.javaparser.JavaParserChanger; import io.codemodder.providers.defectdojo.DefectDojoScan; import io.codemodder.providers.defectdojo.Finding; @@ -34,13 +34,16 @@ public DefectDojoSqlInjectionCodemod( } @Override - public DetectionTool getDetectionTool() { - DetectorRule semgrepSqliRule = - new DetectorRule( - "java.lang.security.audit.sqli.jdbc-sqli.jdbc-sqli", - "java.lang.security.audit.sqli.jdbc-sqli.jdbc-sqli", - null); - return new DetectionTool("DefectDojo", semgrepSqliRule, List.of()); + public String vendorName() { + return "DefectDojo / Semgrep"; + } + + @Override + public DetectorRule getDetectorRule() { + return new DetectorRule( + "java.lang.security.audit.sqli.jdbc-sqli.jdbc-sqli", + "java.lang.security.audit.sqli.jdbc-sqli.jdbc-sqli", + "https://semgrep.dev/r?q=java.lang.security.audit.sqli.jdbc-sqli.jdbc-sqli"); } @Override @@ -54,16 +57,17 @@ public CodemodFileScanningResult visit( return CodemodFileScanningResult.none(); } - List allFindings = new ArrayList<>(); + List unfixedFindings = new ArrayList<>(); List changes = new ArrayList<>(); for (Finding finding : findingsForThisPath) { String id = String.valueOf(finding.getId()); Integer line = finding.getLine(); if (line == null) { - DetectorFinding unfixableFinding = - new DetectorFinding(id, false, "No line number provided"); - allFindings.add(unfixableFinding); + UnfixedFinding unfixableFinding = + new UnfixedFinding( + id, getDetectorRule(), context.path().toString(), null, "No line number provided"); + unfixedFindings.add(unfixableFinding); continue; } @@ -74,33 +78,45 @@ public CodemodFileScanningResult visit( .toList(); if (supportedSqlMethodCallsOnThatLine.isEmpty()) { - DetectorFinding unfixableFinding = - new DetectorFinding(id, false, "No supported SQL methods found on the given line"); - allFindings.add(unfixableFinding); + UnfixedFinding unfixableFinding = + new UnfixedFinding( + id, + getDetectorRule(), + context.path().toString(), + line, + "No supported SQL methods found on the given line"); + unfixedFindings.add(unfixableFinding); continue; } if (supportedSqlMethodCallsOnThatLine.size() > 1) { - DetectorFinding unfixableFinding = - new DetectorFinding( - id, false, "Multiple supported SQL methods found on the given line"); - allFindings.add(unfixableFinding); + UnfixedFinding unfixableFinding = + new UnfixedFinding( + id, + getDetectorRule(), + context.path().toString(), + line, + "Multiple supported SQL methods found on the given line"); + unfixedFindings.add(unfixableFinding); continue; } MethodCallExpr methodCallExpr = supportedSqlMethodCallsOnThatLine.get(0); - if (SQLParameterizerWithCleanup.checkAndFix(methodCallExpr)) { - DetectorFinding fixedFinding = new DetectorFinding(id, true, null); - allFindings.add(fixedFinding); - changes.add(CodemodChange.from(line, "Fixes issue " + id + " by parameterizing SQL")); + FixedFinding fixedFinding = new FixedFinding(id, getDetectorRule()); + changes.add(CodemodChange.from(line, fixedFinding)); } else { - DetectorFinding unfixableFinding = - new DetectorFinding(id, false, "Fixing may have side effects"); - allFindings.add(unfixableFinding); + UnfixedFinding unfixableFinding = + new UnfixedFinding( + id, + getDetectorRule(), + context.path().toString(), + line, + "State changing effects possible or unrecognized code shape"); + unfixedFindings.add(unfixableFinding); } } - return CodemodFileScanningResult.from(changes, allFindings); + return CodemodFileScanningResult.from(changes, unfixedFindings); } } diff --git a/core-codemods/src/main/java/io/codemodder/codemods/DefineConstantForLiteralCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/DefineConstantForLiteralCodemod.java index c15c36767..c62218fc0 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/DefineConstantForLiteralCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/DefineConstantForLiteralCodemod.java @@ -27,7 +27,7 @@ public DefineConstantForLiteralCodemod( } @Override - protected DetectorRule getDetectorRule() { + public DetectorRule getDetectorRule() { return new DetectorRule( "java:S1192", "String literals should not be duplicated", diff --git a/core-codemods/src/main/java/io/codemodder/codemods/FixRedundantStaticOnEnumCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/FixRedundantStaticOnEnumCodemod.java index 38b037ba3..72f17d170 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/FixRedundantStaticOnEnumCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/FixRedundantStaticOnEnumCodemod.java @@ -27,7 +27,7 @@ public FixRedundantStaticOnEnumCodemod( } @Override - protected DetectorRule getDetectorRule() { + public DetectorRule getDetectorRule() { return new DetectorRule( "java:S2786", "Nested `enum`s should not be declared static", diff --git a/core-codemods/src/main/java/io/codemodder/codemods/HardenStringParseToPrimitivesCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/HardenStringParseToPrimitivesCodemod.java index a3d70119a..78353b4d6 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/HardenStringParseToPrimitivesCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/HardenStringParseToPrimitivesCodemod.java @@ -63,7 +63,7 @@ public HardenParseForConstructorChanger( } @Override - protected DetectorRule getDetectorRule() { + public DetectorRule getDetectorRule() { return new DetectorRule( "java:S2130", "Parsing should be used to convert `String`s to primitives", @@ -125,7 +125,7 @@ public HardenParseForValueOfChanger( } @Override - protected DetectorRule getDetectorRule() { + public DetectorRule getDetectorRule() { return new DetectorRule( "java:S2130", "Parsing should be used to convert `String`s to primitives", diff --git a/core-codemods/src/main/java/io/codemodder/codemods/OverridesMatchParentSynchronizationCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/OverridesMatchParentSynchronizationCodemod.java index 2b7d2f288..e1115e205 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/OverridesMatchParentSynchronizationCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/OverridesMatchParentSynchronizationCodemod.java @@ -33,7 +33,7 @@ public OverridesMatchParentSynchronizationCodemod( } @Override - protected DetectorRule getDetectorRule() { + public DetectorRule getDetectorRule() { return new DetectorRule( "java:S3551", "Overrides should match their parent class methods in synchronization", diff --git a/core-codemods/src/main/java/io/codemodder/codemods/RemoveCommentedCodeCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/RemoveCommentedCodeCodemod.java index 08f73a5ef..8cf7cc753 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/RemoveCommentedCodeCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/RemoveCommentedCodeCodemod.java @@ -42,7 +42,7 @@ public RemoveCommentedCodeCodemod( } @Override - protected DetectorRule getDetectorRule() { + public DetectorRule getDetectorRule() { return new DetectorRule( "java:S125", "Sections of code should not be commented out", diff --git a/core-codemods/src/main/java/io/codemodder/codemods/RemoveRedundantVariableCreationCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/RemoveRedundantVariableCreationCodemod.java index e28daf1ec..0a7e71ba6 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/RemoveRedundantVariableCreationCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/RemoveRedundantVariableCreationCodemod.java @@ -29,7 +29,7 @@ public RemoveRedundantVariableCreationCodemod( } @Override - protected DetectorRule getDetectorRule() { + public DetectorRule getDetectorRule() { return new DetectorRule( "java:S1488", "Local variables should not be declared and then immediately returned or thrown", diff --git a/core-codemods/src/main/java/io/codemodder/codemods/RemoveUnusedImportCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/RemoveUnusedImportCodemod.java index ece1653f0..7d5655ff6 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/RemoveUnusedImportCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/RemoveUnusedImportCodemod.java @@ -42,7 +42,7 @@ public RemoveUnusedImportCodemod( } @Override - protected DetectorRule getDetectorRule() { + public DetectorRule getDetectorRule() { return new DetectorRule( "java:S1128", "Unnecessary imports should be removed", diff --git a/core-codemods/src/main/java/io/codemodder/codemods/RemoveUnusedLocalVariableCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/RemoveUnusedLocalVariableCodemod.java index 296079bc7..dc15188e1 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/RemoveUnusedLocalVariableCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/RemoveUnusedLocalVariableCodemod.java @@ -39,7 +39,7 @@ public RemoveUnusedLocalVariableCodemod( } @Override - protected DetectorRule getDetectorRule() { + public DetectorRule getDetectorRule() { return new DetectorRule( "java:S1481", "Unused local variables should be removed", diff --git a/core-codemods/src/main/java/io/codemodder/codemods/RemoveUnusedPrivateMethodCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/RemoveUnusedPrivateMethodCodemod.java index e071c5eef..ca87ada40 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/RemoveUnusedPrivateMethodCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/RemoveUnusedPrivateMethodCodemod.java @@ -30,7 +30,7 @@ public RemoveUnusedPrivateMethodCodemod( } @Override - protected DetectorRule getDetectorRule() { + public DetectorRule getDetectorRule() { return new DetectorRule( "java:S1144", "Unused private methods should be removed", diff --git a/core-codemods/src/main/java/io/codemodder/codemods/RemoveUselessParenthesesCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/RemoveUselessParenthesesCodemod.java index d09cc31c0..5a8013823 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/RemoveUselessParenthesesCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/RemoveUselessParenthesesCodemod.java @@ -28,7 +28,7 @@ public RemoveUselessParenthesesCodemod( } @Override - protected DetectorRule getDetectorRule() { + public DetectorRule getDetectorRule() { return new DetectorRule( "java:S1110", "Redundant pairs of parentheses should be removed", diff --git a/core-codemods/src/main/java/io/codemodder/codemods/ReplaceStreamCollectorsToListCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/ReplaceStreamCollectorsToListCodemod.java index ae901ab0f..bf84151d0 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/ReplaceStreamCollectorsToListCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/ReplaceStreamCollectorsToListCodemod.java @@ -30,7 +30,7 @@ public ReplaceStreamCollectorsToListCodemod( } @Override - protected DetectorRule getDetectorRule() { + public DetectorRule getDetectorRule() { return new DetectorRule( "java:S6204", "`Stream.toList()` should be used instead of `collectors`", diff --git a/core-codemods/src/main/java/io/codemodder/codemods/SimplifyRestControllerAnnotationsCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/SimplifyRestControllerAnnotationsCodemod.java index 5be63aaa6..2a6f6d2b0 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/SimplifyRestControllerAnnotationsCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/SimplifyRestControllerAnnotationsCodemod.java @@ -37,7 +37,7 @@ public SimplifyRestControllerAnnotationsCodemod( } @Override - protected DetectorRule getDetectorRule() { + public DetectorRule getDetectorRule() { return new DetectorRule( "java:S6833", "`@Controller` should be replaced with `@RestController`", diff --git a/core-codemods/src/main/java/io/codemodder/codemods/SubstituteReplaceAllCodemod.java b/core-codemods/src/main/java/io/codemodder/codemods/SubstituteReplaceAllCodemod.java index 59a18ca77..0391bbcc5 100644 --- a/core-codemods/src/main/java/io/codemodder/codemods/SubstituteReplaceAllCodemod.java +++ b/core-codemods/src/main/java/io/codemodder/codemods/SubstituteReplaceAllCodemod.java @@ -26,7 +26,7 @@ public SubstituteReplaceAllCodemod( } @Override - protected DetectorRule getDetectorRule() { + public DetectorRule getDetectorRule() { return new DetectorRule( "java:S5361", "`String#replace` should be preferred to `String#replaceAll`", diff --git a/core-codemods/src/test/java/io/codemodder/codemods/DefectDojoSqlInjectionCodemodTest.java b/core-codemods/src/test/java/io/codemodder/codemods/DefectDojoSqlInjectionCodemodTest.java index 6ee5ac10e..ada627797 100644 --- a/core-codemods/src/test/java/io/codemodder/codemods/DefectDojoSqlInjectionCodemodTest.java +++ b/core-codemods/src/test/java/io/codemodder/codemods/DefectDojoSqlInjectionCodemodTest.java @@ -10,17 +10,21 @@ final class DefectDojoSqlInjectionCodemodTest { @Metadata( codemodType = DefectDojoSqlInjectionCodemod.class, testResourceDir = "defectdojo-sql-injection/SqlInjectionChallenge", + doRetransformTest = false, renameTestFile = "src/main/java/org/owasp/webgoat/lessons/sqlinjection/advanced/SqlInjectionChallenge.java", - dependencies = {}) + dependencies = {}, + expectingFixesAtLines = {69}) final class WebGoatSqlInjectionChallengeTest implements CodemodTestMixin {} @Nested @Metadata( codemodType = DefectDojoSqlInjectionCodemod.class, testResourceDir = "defectdojo-sql-injection/SqlInjectionLesson8", + doRetransformTest = false, renameTestFile = "src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java", - dependencies = {}) + dependencies = {}, + expectingFixesAtLines = {78, 158}) final class WebGoatSqlInjectionLesson8Test implements CodemodTestMixin {} } diff --git a/framework/codemodder-base/build.gradle.kts b/framework/codemodder-base/build.gradle.kts index 85aead210..c29ef28fa 100644 --- a/framework/codemodder-base/build.gradle.kts +++ b/framework/codemodder-base/build.gradle.kts @@ -21,7 +21,7 @@ dependencies { api(libs.java.security.toolkit) api(libs.commons.lang3) - api("io.codemodder:codetf-java:3.2.1") + api("io.codemodder:codetf-java:4.0.0") api(libs.slf4j.api) api(libs.javaparser.core) api(libs.javaparser.symbolsolver.core) diff --git a/framework/codemodder-base/src/main/java/io/codemodder/CodemodChange.java b/framework/codemodder-base/src/main/java/io/codemodder/CodemodChange.java index bed517125..81141eb66 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/CodemodChange.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/CodemodChange.java @@ -1,6 +1,7 @@ package io.codemodder; import io.codemodder.codetf.CodeTFParameter; +import io.codemodder.codetf.FixedFinding; import java.util.List; import java.util.Objects; import java.util.Optional; @@ -15,6 +16,9 @@ public final class CodemodChange { /** Represents the dependencies required by the new code we introduced in this weave. */ private final List dependenciesNeeded; + /** The fixed finding that was addressed by this change. */ + private final List fixedFindings; + /** * Represents the parameters that were used to generate the new code we introduced in this weave. */ @@ -28,6 +32,15 @@ private CodemodChange(final int lineNumber, final List dependenci this.dependenciesNeeded = Objects.requireNonNull(dependenciesNeeded, "dependenciesNeeded"); this.parameters = List.of(); this.description = null; + this.fixedFindings = List.of(); + } + + private CodemodChange(final int lineNumber, final FixedFinding finding) { + this.lineNumber = lineNumber; + this.dependenciesNeeded = List.of(); + this.parameters = List.of(); + this.description = null; + this.fixedFindings = List.of(Objects.requireNonNull(finding)); } private CodemodChange(final int lineNumber, final String description) { @@ -35,6 +48,7 @@ private CodemodChange(final int lineNumber, final String description) { this.dependenciesNeeded = List.of(); this.parameters = List.of(); this.description = Objects.requireNonNull(description); + this.fixedFindings = List.of(); } private CodemodChange(final int lineNumber, final Parameter parameter, final String valueUsed) { @@ -49,6 +63,7 @@ private CodemodChange(final int lineNumber, final Parameter parameter, final Str valueUsed); this.parameters = List.of(codeTFParameter); this.description = null; + this.fixedFindings = List.of(); } @Override @@ -93,6 +108,11 @@ public Optional getDescription() { return Optional.ofNullable(description); } + /** The fixed finding that was addressed by this change. */ + public List getFixedFindings() { + return fixedFindings; + } + /** Builds a weave. */ public static CodemodChange from(final int line, final List dependenciesNeeded) { return new CodemodChange(line, dependenciesNeeded); @@ -114,6 +134,11 @@ public static CodemodChange from(final int line) { return new CodemodChange(line, List.of()); } + public static CodemodChange from(final int line, final FixedFinding finding) { + return new CodemodChange(line, finding); + } + + /** A {@link } */ public static CodemodChange from( final int line, final Parameter parameter, final String valueUsed) { return new CodemodChange(line, parameter, valueUsed); diff --git a/framework/codemodder-base/src/main/java/io/codemodder/CodemodFileScanningResult.java b/framework/codemodder-base/src/main/java/io/codemodder/CodemodFileScanningResult.java index 2021b3a44..834ab3f5d 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/CodemodFileScanningResult.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/CodemodFileScanningResult.java @@ -1,6 +1,6 @@ package io.codemodder; -import io.codemodder.codetf.DetectorFinding; +import io.codemodder.codetf.UnfixedFinding; import java.util.List; /** Represents the result of scanning a file for changes. */ @@ -8,7 +8,7 @@ public interface CodemodFileScanningResult { /** Creates a new instance of {@link CodemodFileScanningResult} from the given values. */ static CodemodFileScanningResult from( - final List changes, final List findings) { + final List changes, final List unfixedFindings) { return new CodemodFileScanningResult() { @Override public List changes() { @@ -16,8 +16,8 @@ public List changes() { } @Override - public List findings() { - return findings; + public List unfixedFindings() { + return unfixedFindings; } }; } @@ -36,5 +36,5 @@ static CodemodFileScanningResult withOnlyChanges(final List chang List changes(); /** Returns the results of the findings that were hoping to be addressed. */ - List findings(); + List unfixedFindings(); } 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 56b2054e6..31d7462f0 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/CompositeJavaParserChanger.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/CompositeJavaParserChanger.java @@ -1,7 +1,7 @@ package io.codemodder; import com.github.javaparser.ast.CompilationUnit; -import io.codemodder.codetf.DetectorFinding; +import io.codemodder.codetf.UnfixedFinding; import io.codemodder.javaparser.JavaParserChanger; import java.util.ArrayList; import java.util.Arrays; @@ -33,16 +33,16 @@ protected CompositeJavaParserChanger(final JavaParserChanger... changers) { public CodemodFileScanningResult visit( final CodemodInvocationContext context, final CompilationUnit cu) { List changes = new ArrayList<>(); - List findings = new ArrayList<>(); + List unfixedFindings = new ArrayList<>(); changers.forEach( changer -> { CodemodFileScanningResult result = changer.visit(context, cu); changes.addAll(result.changes()); - findings.addAll(result.findings()); + unfixedFindings.addAll(result.unfixedFindings()); }); - return CodemodFileScanningResult.from(changes, findings); + return CodemodFileScanningResult.from(changes, unfixedFindings); } @Override 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 3baa26763..760769d1d 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/DefaultCodemodExecutor.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/DefaultCodemodExecutor.java @@ -4,11 +4,7 @@ import com.github.difflib.DiffUtils; import com.github.difflib.UnifiedDiffUtils; -import io.codemodder.codetf.CodeTFChange; -import io.codemodder.codetf.CodeTFChangesetEntry; -import io.codemodder.codetf.CodeTFDiffSide; -import io.codemodder.codetf.CodeTFPackageAction; -import io.codemodder.codetf.CodeTFResult; +import io.codemodder.codetf.*; import io.codemodder.javaparser.JavaParserChanger; import io.codemodder.javaparser.JavaParserCodemodRunner; import io.codemodder.javaparser.JavaParserFacade; @@ -113,6 +109,8 @@ public CodeTFResult execute(final List filePaths) { ExecutorService executor = Executors.newFixedThreadPool(workers); CompletionService service = new ExecutorCompletionService<>(executor); + List unfixedFindings = new CopyOnWriteArrayList<>(); + // for each file, add a task to the completion service for (Path filePath : codemodTargetFiles) { @@ -161,6 +159,8 @@ public CodeTFResult execute(final List filePaths) { } } + unfixedFindings.addAll(codemodFileScanningResult.unfixedFindings()); + } catch (Exception e) { unscannableFiles.add(filePath); log.error("Problem scanning file", e); @@ -182,20 +182,24 @@ public CodeTFResult execute(final List filePaths) { log.error("Problem waiting for scanning threads to exit", e); } + DetectionTool detectionTool = null; + if (codeChanger instanceof FixOnlyCodeChanger fixOnlyCodeChanger) { + detectionTool = new DetectionTool(fixOnlyCodeChanger.vendorName()); + } + CodeTFResult result = new CodeTFResult( codemod.getId(), codeChanger.getSummary(), codeChanger.getDescription(), - codeChanger instanceof FixOnlyCodeChanger fixOnlyCodeChanger - ? fixOnlyCodeChanger.getDetectionTool() - : null, + detectionTool, unscannableFiles.stream() .map(file -> getRelativePath(projectDir, file)) .collect(Collectors.toSet()), codeChanger.getReferences(), emptyMap(), - changeset); + changeset, + unfixedFindings); for (CodeTFProvider provider : codetfProviders) { result = provider.onResultCreated(result); @@ -288,7 +292,8 @@ private CodeTFChange translateCodemodChangetoCodeTFChange( changeDescription, CodeTFDiffSide.LEFT, pkgActions, - codemodChange.getParameters()); + codemodChange.getParameters(), + codemodChange.getFixedFindings()); for (CodeTFProvider provider : codetfProviders) { change = provider.onChangeCreated(filePath, codemod.getId(), change); diff --git a/framework/codemodder-base/src/main/java/io/codemodder/FixOnlyCodeChanger.java b/framework/codemodder-base/src/main/java/io/codemodder/FixOnlyCodeChanger.java index 97fb8ff08..244dbd329 100644 --- a/framework/codemodder-base/src/main/java/io/codemodder/FixOnlyCodeChanger.java +++ b/framework/codemodder-base/src/main/java/io/codemodder/FixOnlyCodeChanger.java @@ -1,6 +1,6 @@ package io.codemodder; -import io.codemodder.codetf.DetectionTool; +import io.codemodder.codetf.DetectorRule; /** * A codemod that only fixes issues and does not perform its own detection, instead relying on @@ -10,6 +10,9 @@ */ public interface FixOnlyCodeChanger { - /** Detection tool metadata for this codemod. */ - DetectionTool getDetectionTool(); + /** Detection tool name. */ + String vendorName(); + + /** A description of the rule. */ + DetectorRule getDetectorRule(); } diff --git a/framework/codemodder-base/src/test/java/io/codemodder/DefaultCodemodExecutorTest.java b/framework/codemodder-base/src/test/java/io/codemodder/DefaultCodemodExecutorTest.java index 8fce1adf6..a6860cdd0 100644 --- a/framework/codemodder-base/src/test/java/io/codemodder/DefaultCodemodExecutorTest.java +++ b/framework/codemodder-base/src/test/java/io/codemodder/DefaultCodemodExecutorTest.java @@ -12,6 +12,7 @@ import com.github.difflib.UnifiedDiffUtils; import com.github.javaparser.JavaParser; import com.github.javaparser.ast.CompilationUnit; +import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration; import com.github.javaparser.ast.body.MethodDeclaration; import com.github.javaparser.ast.stmt.BlockStmt; import io.codemodder.codetf.*; @@ -136,6 +137,7 @@ public DependencyUpdateResult updateDependencies( "updated deps", CodeTFDiffSide.LEFT, List.of(packageAddResult), + List.of(), List.of()); CodeTFChangesetEntry entry = new CodeTFChangesetEntry("deps.txt", diff, List.of(change)); List changes = List.of(entry); @@ -160,6 +162,36 @@ void it_generates_single_codemod_codetf() { assertThat(changeset.get(0)).satisfies(DefaultCodemodExecutorTest::isJavaFile1ChangedCorrectly); } + @Test + void it_collects_fixonly_metadata() { + executor = + new DefaultCodemodExecutor( + repoDir, + includesEverything, + new CodemodIdPair("thirdparty:java/thing", new ProvidesRemediationStuffCodemod()), + List.of(), + List.of(), + fileCache, + javaParserFacade, + encodingDetector, + -1, + -1, + -1); + + CodeTFResult result = executor.execute(List.of(javaFile1)); + DetectionTool detectionTool = result.getDetectionTool(); + assertThat(detectionTool.getName()).isEqualTo("acme"); + + assertThat(result.getFailedFiles()).isEmpty(); + + List unfixedFindings = result.getUnfixedFindings(); + assertThat(unfixedFindings) + .containsExactlyElementsOf(ProvidesRemediationStuffCodemod.unfixedFindings); + + CodeTFChange change = result.getChangeset().get(0).getChanges().get(0); + assertThat(change.getFixedFindings()).containsOnly(ProvidesRemediationStuffCodemod.finding); + } + @Test void it_allows_codetf_customization() { @@ -718,6 +750,60 @@ public String getIndividualChangeDescription(final Path filePath, final CodemodC } } + private static class ProvidesRemediationStuffCodemod extends JavaParserChanger + implements FixOnlyCodeChanger { + + ProvidesRemediationStuffCodemod() { + super(new EmptyReporter()); + } + + @Override + public CodemodFileScanningResult visit( + final CodemodInvocationContext context, final CompilationUnit cu) { + cu.findAll(ClassOrInterfaceDeclaration.class).get(0).addExtendedType("foo"); + CodemodChange change = CodemodChange.from(10, finding); + return CodemodFileScanningResult.from(List.of(change), unfixedFindings); + } + + private static final DetectorRule rule = + new DetectorRule("acme_rule_id", "Find Thing", "https://acme.com/rules/thing"); + + private static final FixedFinding finding = new FixedFinding("id-4", rule); + + private static final List unfixedFindings = + List.of(new UnfixedFinding("rule-2", rule, "/path/thing.java", 15, "couldn't find thing")); + + @Override + public String getDescription() { + return "injects-dependency-2-description"; + } + + @Override + public String getSummary() { + return "injects-dependency-2-summary"; + } + + @Override + public List getReferences() { + return List.of(new CodeTFReference("https://dep2.com/", "https://dep2.com/")); + } + + @Override + public String getIndividualChangeDescription(final Path filePath, final CodemodChange change) { + return "injects-dependency-2-change"; + } + + @Override + public String vendorName() { + return "acme"; + } + + @Override + public DetectorRule getDetectorRule() { + return rule; + } + } + private static final DependencyGAV dependency1 = DependencyGAV.createDefault("org.spring", "dep1", "1.1.1"); private static final DependencyGAV dependency2 = diff --git a/framework/codemodder-testutils/src/main/java/io/codemodder/testutils/CodemodTestMixin.java b/framework/codemodder-testutils/src/main/java/io/codemodder/testutils/CodemodTestMixin.java index 3fa6cd8ec..62a56e9d9 100644 --- a/framework/codemodder-testutils/src/main/java/io/codemodder/testutils/CodemodTestMixin.java +++ b/framework/codemodder-testutils/src/main/java/io/codemodder/testutils/CodemodTestMixin.java @@ -7,9 +7,7 @@ import com.github.difflib.DiffUtils; import com.github.difflib.patch.Patch; import io.codemodder.*; -import io.codemodder.codetf.CodeTFChange; -import io.codemodder.codetf.CodeTFChangesetEntry; -import io.codemodder.codetf.CodeTFResult; +import io.codemodder.codetf.*; import io.codemodder.javaparser.JavaParserFacade; import io.codemodder.javaparser.JavaParserFactory; import java.io.IOException; @@ -76,7 +74,9 @@ default Stream generateTestCases(@TempDir final Path tmpDir) throws path, dependencies, projectProviders, - metadata.doRetransformTest()); + metadata.doRetransformTest(), + metadata.expectingFixesAtLines(), + metadata.expectingFailedFixesAtLines()); return DynamicTest.stream(inputStream, displayNameGenerator, testExecutor); } @@ -89,7 +89,9 @@ private void verifyCodemod( final Path before, final List dependenciesExpected, final List projectProviders, - final boolean doRetransformTest) + final boolean doRetransformTest, + final int[] expectedFixLines, + final int[] expectingFailedFixesAtLines) throws IOException { // create a copy of the test file in the temp directory to serve as our "repository" @@ -184,6 +186,19 @@ private void verifyCodemod( assertThat(change.getDescription(), is(not(blankOrNullString()))); } + List changes = + changeset.stream().map(CodeTFChangesetEntry::getChanges).flatMap(List::stream).toList(); + + for (int expectedFixLine : expectedFixLines) { + assertThat(changes.stream().anyMatch(c -> c.getLineNumber() == expectedFixLine), is(true)); + } + + List unfixedFindings = result.getUnfixedFindings(); + for (int expectedFailedFixLine : expectingFailedFixesAtLines) { + assertThat( + unfixedFindings.stream().noneMatch(c -> c.getLine() == expectedFailedFixLine), is(true)); + } + // make sure that some of the basics are being reported assertThat(result.getSummary(), is(not(blankOrNullString()))); assertThat(result.getDescription(), is(not(blankOrNullString()))); diff --git a/framework/codemodder-testutils/src/main/java/io/codemodder/testutils/Metadata.java b/framework/codemodder-testutils/src/main/java/io/codemodder/testutils/Metadata.java index 8304e2422..cf3640c1b 100644 --- a/framework/codemodder-testutils/src/main/java/io/codemodder/testutils/Metadata.java +++ b/framework/codemodder-testutils/src/main/java/io/codemodder/testutils/Metadata.java @@ -42,4 +42,10 @@ * HardenXStreamCodemodTestProjectProvider} */ Class[] projectProviders() default {}; + + /** The expected fix metadata that the codemod should report. */ + int[] expectingFixesAtLines() default {}; + + /** The expected failed fix metadata that the codemod should report. */ + int[] expectingFailedFixesAtLines() default {}; } diff --git a/gradle/build-settings/src/main/kotlin/io.codemodder.repositories.settings.gradle.kts b/gradle/build-settings/src/main/kotlin/io.codemodder.repositories.settings.gradle.kts index 9299385fc..717c5efe5 100644 --- a/gradle/build-settings/src/main/kotlin/io.codemodder.repositories.settings.gradle.kts +++ b/gradle/build-settings/src/main/kotlin/io.codemodder.repositories.settings.gradle.kts @@ -3,6 +3,7 @@ dependencyResolutionManagement { @Suppress("UnstableApiUsage") repositories { mavenCentral() + mavenLocal() gradlePluginPortal() } } diff --git a/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/CodeTFGenerator.java b/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/CodeTFGenerator.java index ec7310b57..f58c79c52 100644 --- a/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/CodeTFGenerator.java +++ b/plugins/codemodder-plugin-maven/src/main/java/io/codemodder/plugins/maven/CodeTFGenerator.java @@ -68,7 +68,13 @@ CodeTFChangesetEntry getChanges( // Use RIGHT side as the diff side for now, as we are only adding dependencies. final CodeTFChange change = new CodeTFChange( - position, properties, description, CodeTFDiffSide.RIGHT, List.of(), List.of()); + position, + properties, + description, + CodeTFDiffSide.RIGHT, + List.of(), + List.of(), + List.of()); final List patchDiff = UnifiedDiffUtils.generateUnifiedDiff( 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 f836da60e..42117b9eb 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 @@ -4,8 +4,6 @@ import com.github.javaparser.ast.CompilationUnit; import com.github.javaparser.ast.Node; import io.codemodder.*; -import io.codemodder.codetf.DetectionTool; -import io.codemodder.codetf.DetectorRule; import io.codemodder.javaparser.ChangesResult; import io.codemodder.javaparser.JavaParserChanger; import io.codemodder.providers.sonar.api.Issue; @@ -98,11 +96,9 @@ public boolean shouldRun() { return ruleIssues.hasResults(); } - protected abstract DetectorRule getDetectorRule(); - @Override - public DetectionTool getDetectionTool() { - return new DetectionTool("Sonar", getDetectorRule(), List.of()); + public String vendorName() { + return "Sonar"; } /**