Added remediators and improved method searching flexibility (#437)

Author: nahsra

core-codemods/src/main/java/io/codemodder/codemods/DefectDojoSqlInjectionCodemod.java

@@ -59,6 +59,7 @@ public CodemodFileScanningResult visit(
         detectorRule(),
         findingsForThisPath,
         finding -> String.valueOf(finding.getId()),
-        Finding::getLine);
+        Finding::getLine,
+        f -> null);
   }
 }

core-codemods/src/main/java/io/codemodder/codemods/SonarSQLInjectionCodemod.java

@@ -50,6 +50,7 @@ public CodemodFileScanningResult visit(
         detectorRule(),
         hotspotsForFile,
         SonarFinding::getKey,
-        SonarFinding::getLine);
+        i -> i.getTextRange() != null ? i.getTextRange().getStartLine() : i.getLine(),
+        i -> i.getTextRange() != null ? i.getTextRange().getEndLine() : null);
   }
 }

core-codemods/src/main/java/io/codemodder/codemods/SonarUnsafeReflectionRemediationCodemod.java

@@ -49,7 +49,8 @@ public CodemodFileScanningResult visit(
         detectorRule(),
         issues.getResultsByPath(context.path()),
         Issue::getKey,
-        Issue::getLine,
+        i -> i.getTextRange() != null ? i.getTextRange().getStartLine() : i.getLine(),
+        i -> i.getTextRange() != null ? i.getTextRange().getEndLine() : null,
         i -> i.getTextRange().getStartOffset());
   }
 }

core-codemods/src/main/java/io/codemodder/codemods/SonarXXECodemod.java

@@ -49,7 +49,7 @@ public CodemodFileScanningResult visit(
         detectorRule(),
         issuesForFile,
         SonarFinding::getKey,
-        SonarFinding::getLine,
+        f -> f.getTextRange() != null ? f.getTextRange().getStartLine() : f.getLine(),
         f -> f.getTextRange().getStartOffset());
   }
 }

framework/codemodder-base/src/main/java/io/codemodder/javaparser/ChangesResult.java

@@ -5,6 +5,8 @@
 
 /** Represents the result of changes made during parsing. */
 public interface ChangesResult {
+
+  /** Returns true if changes were applied. */
   boolean areChangesApplied();
 
   List<DependencyGAV> getDependenciesRequired();

framework/codemodder-base/src/main/java/io/codemodder/remediation/DefaultFixCandidateSearcher.java

@@ -2,23 +2,23 @@
 
 import com.github.javaparser.Position;
 import com.github.javaparser.ast.CompilationUnit;
+import com.github.javaparser.ast.Node;
 import com.github.javaparser.ast.expr.MethodCallExpr;
+import com.github.javaparser.ast.expr.ObjectCreationExpr;
 import io.codemodder.codetf.DetectorRule;
 import io.codemodder.codetf.UnfixedFinding;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 import java.util.function.Function;
 import java.util.function.Predicate;
+import org.jetbrains.annotations.VisibleForTesting;
 
 final class DefaultFixCandidateSearcher<T> implements FixCandidateSearcher<T> {
 
-  private final List<Predicate<MethodCallExpr>> matchers;
+  private final List<Predicate<MethodOrConstructor>> matchers;
   private final String methodName;
 
   DefaultFixCandidateSearcher(
-      final String methodName, final List<Predicate<MethodCallExpr>> matchers) {
+      final String methodName, final List<Predicate<MethodOrConstructor>> matchers) {
     this.methodName = methodName;
     this.matchers = matchers;
   }
@@ -30,50 +30,61 @@ public FixCandidateSearchResults<T> search(
       final DetectorRule rule,
       final List<T> issuesForFile,
       final Function<T, String> getKey,
-      final Function<T, Integer> getLine,
+      final Function<T, Integer> getStartLine,
+      final Function<T, Integer> getEndLine,
       final Function<T, Integer> getColumn) {
 
     List<UnfixedFinding> unfixedFindings = new ArrayList<>();
-    List<MethodCallExpr> calls =
-        cu.findAll(MethodCallExpr.class).stream()
-            .filter(
-                mce ->
-                    mce.getRange()
-                        .isPresent()) // don't find calls we may have added -- you can pick these
-            // out by them not having a range yet
-            .filter(mce -> methodName == null || methodName.equals(mce.getNameAsString()))
+
+    List<MethodOrConstructor> calls =
+        cu.findAll(Node.class).stream()
+            // limit to just the interesting nodes for us
+            .filter(n -> n instanceof MethodCallExpr || n instanceof ObjectCreationExpr)
+            // turn them into our convenience type
+            .map(MethodOrConstructor::new)
+            // don't find calls we may have added -- you can pick these out by them not having a
+            // range yet
+            .filter(MethodOrConstructor::hasRange)
+            // filter by method name if one is provided in the search
+            .filter(mce -> methodName == null || mce.isMethodCallWithName(methodName))
+            // filter by matchers
             .filter(mce -> matchers.stream().allMatch(m -> m.test(mce)))
             .toList();
 
-    Map<MethodCallExpr, List<T>> fixCandidateToIssueMapping = new HashMap<>();
+    Map<MethodOrConstructor, List<T>> fixCandidateToIssueMapping = new HashMap<>();
 
     for (T issue : issuesForFile) {
       String findingId = getKey.apply(issue);
-      int line = getLine.apply(issue);
+      int issueStartLine = getStartLine.apply(issue);
+      Integer issueEndLine = getEndLine.apply(issue);
       Integer column = getColumn.apply(issue);
-      List<MethodCallExpr> callsForIssue =
+      List<MethodOrConstructor> callsForIssue =
           calls.stream()
-              .filter(mce -> mce.getRange().isPresent())
-              .filter(mce -> mce.getRange().get().begin.line == line)
+              .filter(MethodOrConstructor::hasRange)
+              .filter(
+                  mce -> {
+                    int callStartLine = mce.getRange().begin.line;
+                    return matches(issueStartLine, issueEndLine, callStartLine);
+                  })
               .toList();
       if (callsForIssue.size() > 1 && column != null) {
         callsForIssue =
             callsForIssue.stream()
-                .filter(mce -> mce.getRange().get().contains(new Position(line, column)))
+                .filter(mce -> mce.getRange().contains(new Position(issueStartLine, column)))
                 .toList();
       }
       if (callsForIssue.isEmpty()) {
         unfixedFindings.add(
             new UnfixedFinding(
-                findingId, rule, path, line, RemediationMessages.noCallsAtThatLocation));
+                findingId, rule, path, issueStartLine, RemediationMessages.noCallsAtThatLocation));
         continue;
       } else if (callsForIssue.size() > 1) {
         unfixedFindings.add(
             new UnfixedFinding(
-                findingId, rule, path, line, RemediationMessages.multipleCallsFound));
+                findingId, rule, path, issueStartLine, RemediationMessages.multipleCallsFound));
         continue;
       }
-      MethodCallExpr call = callsForIssue.get(0);
+      MethodOrConstructor call = callsForIssue.get(0);
       fixCandidateToIssueMapping.computeIfAbsent(call, k -> new ArrayList<>()).add(issue);
     }
 
@@ -94,4 +105,19 @@ public List<FixCandidate<T>> fixCandidates() {
       }
     };
   }
+
+  @VisibleForTesting
+  static boolean matches(
+      final int issueStartLine, final Integer issueEndLine, final int startCallLine) {
+    // if the issue spans multiple lines, the call must be within that range
+    if (issueEndLine != null) {
+      return isInBetween(startCallLine, issueStartLine, issueEndLine);
+    }
+    // if the issue is on a single line, the call must be on that line
+    return startCallLine == issueStartLine;
+  }
+
+  private static boolean isInBetween(int number, int lowerBound, int upperBound) {
+    return number >= lowerBound && number <= upperBound;
+  }
 }

framework/codemodder-base/src/main/java/io/codemodder/remediation/FixCandidate.java

@@ -1,14 +1,13 @@
 package io.codemodder.remediation;
 
-import com.github.javaparser.ast.expr.MethodCallExpr;
 import java.util.List;
 import java.util.Objects;
 
 /** The potential fix location. */
-public record FixCandidate<T>(MethodCallExpr methodCall, List<T> issues) {
+public record FixCandidate<T>(MethodOrConstructor call, List<T> issues) {
 
   public FixCandidate {
-    Objects.requireNonNull(methodCall);
+    Objects.requireNonNull(call);
     Objects.requireNonNull(issues);
     if (issues.isEmpty()) {
       throw new IllegalArgumentException("issues cannot be empty");

framework/codemodder-base/src/main/java/io/codemodder/remediation/FixCandidateSearcher.java

@@ -1,7 +1,6 @@
 package io.codemodder.remediation;
 
 import com.github.javaparser.ast.CompilationUnit;
-import com.github.javaparser.ast.expr.MethodCallExpr;
 import io.codemodder.codetf.DetectorRule;
 import java.util.ArrayList;
 import java.util.List;
@@ -19,13 +18,14 @@ FixCandidateSearchResults<T> search(
       DetectorRule rule,
       List<T> issuesForFile,
       Function<T, String> getKey,
-      Function<T, Integer> getLine,
+      Function<T, Integer> getStartLine,
+      Function<T, Integer> getEndLine,
       Function<T, Integer> getColumn);
 
   /** Builder for {@link FixCandidateSearcher}. */
   final class Builder<T> {
     private String methodName;
-    private final List<Predicate<MethodCallExpr>> methodMatchers;
+    private final List<Predicate<MethodOrConstructor>> methodMatchers;
 
     public Builder() {
       this.methodMatchers = new ArrayList<>();
@@ -36,7 +36,7 @@ public Builder<T> withMethodName(final String methodName) {
       return this;
     }
 
-    public Builder<T> withMatcher(final Predicate<MethodCallExpr> methodMatcher) {
+    public Builder<T> withMatcher(final Predicate<MethodOrConstructor> methodMatcher) {
       this.methodMatchers.add(Objects.requireNonNull(methodMatcher));
       return this;
     }

framework/codemodder-base/src/main/java/io/codemodder/remediation/GenericRemediationMetadata.java

@@ -11,6 +11,7 @@ public enum GenericRemediationMetadata {
   HEADER_INJECTION("header-injection"),
   REFLECTION_INJECTION("reflection-injection"),
   DESERIALIZATION("java-deserialization"),
+  MISSING_SECURE_FLAG("missing-secure-flag"),
   SQL_INJECTION("sql-injection");
 
   private final CodemodReporterStrategy reporter;

framework/codemodder-base/src/main/java/io/codemodder/remediation/MethodOrConstructor.java

@@ -0,0 +1,81 @@
+package io.codemodder.remediation;
+
+import com.github.javaparser.Range;
+import com.github.javaparser.ast.Node;
+import com.github.javaparser.ast.NodeList;
+import com.github.javaparser.ast.expr.MethodCallExpr;
+import com.github.javaparser.ast.expr.ObjectCreationExpr;
+import com.github.javaparser.ast.nodeTypes.NodeWithArguments;
+import java.util.Collection;
+import java.util.Objects;
+
+/** Convenience type to represent a method or constructor call in our APIs that work with both. */
+public record MethodOrConstructor(Node node) {
+
+  public MethodOrConstructor {
+    Objects.requireNonNull(node);
+  }
+
+  /** Return the wrapped node as a {@link Node}. */
+  public Node asNode() {
+    return this.node;
+  }
+
+  /** This assumes a range is present, and explodes if it doesn't. */
+  public Range getRange() {
+    return this.asNode().getRange().orElseThrow();
+  }
+
+  /** Return the wrapped node as a {@link com.github.javaparser.ast.nodeTypes.NodeWithArguments}. */
+  public NodeWithArguments<?> asNodeWithArguments() {
+    return (NodeWithArguments<?>) this.node;
+  }
+
+  /** Get the arguments for the call. */
+  public NodeList<?> getArguments() {
+    return this.asNodeWithArguments().getArguments();
+  }
+
+  /** Return true if this is a constructor call. */
+  public boolean isConstructor() {
+    return this.node instanceof ObjectCreationExpr;
+  }
+
+  /** Return true if this is a method call. */
+  public boolean isMethodCall() {
+    return this.node instanceof MethodCallExpr;
+  }
+
+  /** Return true if this is a method call with a scope. */
+  public boolean isMethodCallWithScope() {
+    return this.node instanceof MethodCallExpr mce && mce.getScope().isPresent();
+  }
+
+  /** Return true if this is a constructor call for the given type. */
+  public boolean isConstructorForType(final String type) {
+    return this.node instanceof ObjectCreationExpr oce && oce.getTypeAsString().equals(type);
+  }
+
+  /** Return true if the node has a range, meaning it was not added by us. */
+  public boolean hasRange() {
+    return this.asNode().getRange().isPresent();
+  }
+
+  /** Return true if this is a method call and it has the given name. */
+  public boolean isMethodCallWithName(final String name) {
+    return this.isMethodCall() && ((MethodCallExpr) this.node).getNameAsString().equals(name);
+  }
+
+  /** Return true if this is a method call and it has one of the given names. */
+  public boolean isMethodCallWithNameIn(final Collection<String> names) {
+    return this.isMethodCall() && names.contains(((MethodCallExpr) this.node).getNameAsString());
+  }
+
+  /** Return the wrapped node as a {@link MethodCallExpr} or blow up. */
+  public MethodCallExpr asMethodCall() {
+    if (this.isMethodCall()) {
+      return (MethodCallExpr) this.node;
+    }
+    throw new IllegalStateException("Not a method call");
+  }
+}