Skip to content

Commit 08a0915

Browse files
nahsranahsra
authored
Add overlapping fix logic (#409)
This adds more the ability to group fixes by location and more tests for all the fix candidate searcher logic.
1 parent 7dc962e commit 08a0915

File tree

10 files changed

+219
-86
lines changed

10 files changed

+219
-86
lines changed

‎core-codemods/src/main/java/io/codemodder/codemods/SQLParameterizerCodemod.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import io.codemodder.javaparser.JavaParserChanger;
77
import java.util.List;
88
import java.util.Optional;
9-
import java.util.stream.Collectors;
109

1110
/** Parameterizes SQL statements in the JDBC API. */
1211
@Codemod(
@@ -29,7 +28,7 @@ public CodemodFileScanningResult visit(
2928
List<CodemodChange> changes =
3029
cu.findAll(MethodCallExpr.class).stream()
3130
.flatMap(mce -> onNodeFound(mce).stream())
32-
.collect(Collectors.toList());
31+
.toList();
3332
return CodemodFileScanningResult.withOnlyChanges(changes);
3433
}
3534
}

‎core-codemods/src/main/java/io/codemodder/codemods/util/DefaultJavaParserSQLInjectionRemediatorStrategy.java‎

Lines changed: 45 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@
99
import io.codemodder.codetf.DetectorRule;
1010
import io.codemodder.codetf.FixedFinding;
1111
import io.codemodder.codetf.UnfixedFinding;
12+
import io.codemodder.remediation.FixCandidate;
13+
import io.codemodder.remediation.FixCandidateSearchResults;
14+
import io.codemodder.remediation.FixCandidateSearcher;
1215
import java.util.ArrayList;
1316
import java.util.Collection;
1417
import java.util.List;
@@ -42,7 +45,20 @@ public <T> CodemodFileScanningResult remediateAll(
4245
final Function<T, String> findingIdExtractor,
4346
final Function<T, Integer> findingLineExtractor) {
4447

45-
final List<MethodCallExpr> allMethodCalls = cu.findAll(MethodCallExpr.class);
48+
FixCandidateSearcher<T> searcher =
49+
new FixCandidateSearcher.Builder<T>()
50+
.withMatcher(SQLParameterizer::isSupportedJdbcMethodCall)
51+
.build();
52+
53+
FixCandidateSearchResults<T> results =
54+
searcher.search(
55+
cu,
56+
path,
57+
detectorRule,
58+
new ArrayList<>(findingsForPath),
59+
findingIdExtractor,
60+
findingLineExtractor,
61+
f -> null);
4662

4763
if (findingsForPath.isEmpty()) {
4864
return CodemodFileScanningResult.none();
@@ -51,55 +67,41 @@ public <T> CodemodFileScanningResult remediateAll(
5167
final List<UnfixedFinding> unfixedFindings = new ArrayList<>();
5268
final List<CodemodChange> changes = new ArrayList<>();
5369

54-
for (T finding : findingsForPath) {
55-
final String id = findingIdExtractor.apply(finding);
56-
final Integer line = findingLineExtractor.apply(finding);
70+
for (FixCandidate<T> fixCandidate : results.fixCandidates()) {
71+
List<T> issues = fixCandidate.issues();
72+
Integer line = findingLineExtractor.apply(issues.get(0));
5773

5874
if (line == null) {
59-
final UnfixedFinding unfixableFinding =
60-
new UnfixedFinding(id, detectorRule, path, null, "No line number provided");
61-
unfixedFindings.add(unfixableFinding);
62-
continue;
63-
}
64-
65-
final List<MethodCallExpr> supportedSqlMethodCallsOnThatLine =
66-
allMethodCalls.stream()
67-
.filter(methodCallExpr -> methodCallExpr.getRange().get().begin.line == line)
68-
.filter(SQLParameterizer::isSupportedJdbcMethodCall)
69-
.toList();
70-
71-
if (supportedSqlMethodCallsOnThatLine.isEmpty()) {
72-
final UnfixedFinding unfixableFinding =
73-
new UnfixedFinding(
74-
id, detectorRule, path, line, "No supported SQL methods found on the given line");
75-
unfixedFindings.add(unfixableFinding);
76-
continue;
77-
}
78-
79-
if (supportedSqlMethodCallsOnThatLine.size() > 1) {
80-
final UnfixedFinding unfixableFinding =
81-
new UnfixedFinding(
82-
id,
83-
detectorRule,
84-
path,
85-
line,
86-
"Multiple supported SQL methods found on the given line");
87-
unfixedFindings.add(unfixableFinding);
75+
issues.forEach(
76+
issue -> {
77+
final String id = findingIdExtractor.apply(issue);
78+
final UnfixedFinding unfixableFinding =
79+
new UnfixedFinding(id, detectorRule, path, null, "No line number provided");
80+
unfixedFindings.add(unfixableFinding);
81+
});
8882
continue;
8983
}
9084

91-
final MethodCallExpr methodCallExpr = supportedSqlMethodCallsOnThatLine.get(0);
85+
final MethodCallExpr methodCallExpr = fixCandidate.methodCall();
9286
if (SQLParameterizerWithCleanup.checkAndFix(methodCallExpr)) {
93-
changes.add(CodemodChange.from(line, new FixedFinding(id, detectorRule)));
87+
issues.forEach(
88+
issue -> {
89+
final String id = findingIdExtractor.apply(issue);
90+
changes.add(CodemodChange.from(line, new FixedFinding(id, detectorRule)));
91+
});
9492
} else {
95-
final UnfixedFinding unfixableFinding =
96-
new UnfixedFinding(
97-
id,
98-
detectorRule,
99-
path,
100-
line,
101-
"State changing effects possible or unrecognized code shape");
102-
unfixedFindings.add(unfixableFinding);
93+
issues.forEach(
94+
issue -> {
95+
final String id = findingIdExtractor.apply(issue);
96+
final UnfixedFinding unfixableFinding =
97+
new UnfixedFinding(
98+
id,
99+
detectorRule,
100+
path,
101+
line,
102+
"State changing effects possible or unrecognized code shape");
103+
unfixedFindings.add(unfixableFinding);
104+
});
103105
}
104106
}
105107

‎core-codemods/src/test/java/io/codemodder/codemods/SensitiveDataLoggingCodemodTest.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
import io.codemodder.plugins.llm.test.LLMVerifyingCodemodTestMixin;
44
import io.codemodder.plugins.llm.test.OpenAIIntegrationTest;
55
import io.codemodder.testutils.Metadata;
6+
import org.junit.jupiter.api.Disabled;
67

8+
@Disabled
79
@Metadata(
810
codemodType = SensitiveDataLoggingCodemod.class,
911
testResourceDir = "sensitive-data-logging",

‎framework/codemodder-base/src/main/java/io/codemodder/CodemodChange.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,11 @@ public static CodemodChange from(
163163
return new CodemodChange(line, dependencyGAVS, List.of(finding));
164164
}
165165

166+
public static CodemodChange from(
167+
final int line, final List<DependencyGAV> dependencyGAVS, final List<FixedFinding> finding) {
168+
return new CodemodChange(line, dependencyGAVS, finding);
169+
}
170+
166171
public static CodemodChange from(
167172
final int line, final String description, final FixedFinding finding) {
168173
return new CodemodChange(line, description, List.of(finding));

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
import io.codemodder.codetf.DetectorRule;
77
import io.codemodder.codetf.UnfixedFinding;
88
import java.util.ArrayList;
9+
import java.util.HashMap;
910
import java.util.List;
11+
import java.util.Map;
1012
import java.util.function.Function;
1113
import java.util.function.Predicate;
1214

@@ -32,7 +34,6 @@ public FixCandidateSearchResults<T> search(
3234
final Function<T, Integer> getColumn) {
3335

3436
List<UnfixedFinding> unfixedFindings = new ArrayList<>();
35-
List<FixCandidate<T>> fixCandidates = new ArrayList<>();
3637
List<MethodCallExpr> calls =
3738
cu.findAll(MethodCallExpr.class).stream()
3839
.filter(
@@ -44,6 +45,8 @@ public FixCandidateSearchResults<T> search(
4445
.filter(mce -> matchers.stream().allMatch(m -> m.test(mce)))
4546
.toList();
4647

48+
Map<MethodCallExpr, List<T>> fixCandidateToIssueMapping = new HashMap<>();
49+
4750
for (T issue : issuesForFile) {
4851
String findingId = getKey.apply(issue);
4952
int line = getLine.apply(issue);
@@ -71,9 +74,14 @@ public FixCandidateSearchResults<T> search(
7174
continue;
7275
}
7376
MethodCallExpr call = callsForIssue.get(0);
74-
fixCandidates.add(new FixCandidate<>(call, issue));
77+
fixCandidateToIssueMapping.computeIfAbsent(call, k -> new ArrayList<>()).add(issue);
7578
}
7679

80+
List<FixCandidate<T>> fixCandidates =
81+
fixCandidateToIssueMapping.entrySet().stream()
82+
.map(entry -> new FixCandidate<>(entry.getKey(), entry.getValue()))
83+
.toList();
84+
7785
return new FixCandidateSearchResults<T>() {
7886
@Override
7987
public List<UnfixedFinding> unfixableFindings() {

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
11
package io.codemodder.remediation;
22

33
import com.github.javaparser.ast.expr.MethodCallExpr;
4+
import java.util.List;
45
import java.util.Objects;
56

67
/** The potential fix location. */
7-
public record FixCandidate<T>(MethodCallExpr methodCall, T issue) {
8+
public record FixCandidate<T>(MethodCallExpr methodCall, List<T> issues) {
89

910
public FixCandidate {
1011
Objects.requireNonNull(methodCall);
11-
Objects.requireNonNull(issue);
12+
Objects.requireNonNull(issues);
13+
if (issues.isEmpty()) {
14+
throw new IllegalArgumentException("issues cannot be empty");
15+
}
1216
}
1317
}

‎framework/codemodder-base/src/main/java/io/codemodder/remediation/headerinjection/DefaultHeaderInjectionRemediator.java‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,7 @@ public <T> CodemodFileScanningResult remediateAll(
6363

6464
List<CodemodChange> changes = new ArrayList<>();
6565
for (FixCandidate<T> fixCandidate : results.fixCandidates()) {
66-
T issue = fixCandidate.issue();
67-
String findingId = getKey.apply(issue);
68-
int line = getLine.apply(issue);
66+
List<T> issues = fixCandidate.issues();
6967

7068
MethodCallExpr setHeaderCall = fixCandidate.methodCall();
7169
Expression headerValueArgument = setHeaderCall.getArgument(1);
@@ -94,7 +92,14 @@ public <T> CodemodFileScanningResult remediateAll(
9492
parentClass.addMember(fixMethod);
9593
}
9694
}
97-
changes.add(CodemodChange.from(line, new FixedFinding(findingId, detectorRule)));
95+
96+
// all the line numbers should be the same, so we just grab the first one
97+
int line = getLine.apply(fixCandidate.issues().get(0));
98+
List<FixedFinding> fixedFindings =
99+
issues.stream()
100+
.map(issue -> new FixedFinding(getKey.apply(issue), detectorRule))
101+
.toList();
102+
changes.add(CodemodChange.from(line, List.of(), fixedFindings));
98103
}
99104

100105
return CodemodFileScanningResult.from(changes, results.unfixableFindings());

‎framework/codemodder-base/src/main/java/io/codemodder/remediation/jndiinjection/DefaultJNDIInjectionRemediator.java‎

Lines changed: 61 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -61,38 +61,54 @@ public <T> CodemodFileScanningResult remediateAll(
6161
List<CodemodChange> changes = new ArrayList<>();
6262

6363
for (FixCandidate<T> fixCandidate : results.fixCandidates()) {
64-
T issue = fixCandidate.issue();
65-
String findingId = getKey.apply(issue);
66-
int line = getLine.apply(issue);
64+
List<T> issues = fixCandidate.issues();
65+
int line = getLine.apply(issues.get(0));
6766

6867
MethodCallExpr lookupCall = fixCandidate.methodCall();
6968
// get the parent method of the lookup() call
7069
Optional<MethodDeclaration> parentMethod = lookupCall.findAncestor(MethodDeclaration.class);
7170
if (parentMethod.isEmpty()) {
72-
UnfixedFinding unfixedFinding =
73-
new UnfixedFinding(
74-
findingId, detectorRule, path, line, "No method found around lookup call");
75-
unfixedFindings.add(unfixedFinding);
71+
issues.stream()
72+
.map(getKey)
73+
.map(
74+
findingId ->
75+
new UnfixedFinding(
76+
findingId, detectorRule, path, line, "No method found around lookup call"))
77+
.forEach(unfixedFindings::add);
7678
continue;
7779
}
7880

7981
// confirm its a concrete type -- can't add validation method to
8082
ClassOrInterfaceDeclaration parentClass =
8183
parentMethod.get().findAncestor(ClassOrInterfaceDeclaration.class).get();
8284
if (parentClass.isInterface()) {
83-
UnfixedFinding unfixedFinding =
84-
new UnfixedFinding(
85-
findingId, detectorRule, path, line, "Cannot add validation method to interface");
86-
unfixedFindings.add(unfixedFinding);
85+
issues.stream()
86+
.map(getKey)
87+
.map(
88+
findingId ->
89+
new UnfixedFinding(
90+
findingId,
91+
detectorRule,
92+
path,
93+
line,
94+
"Cannot add validation method to interface"))
95+
.forEach(unfixedFindings::add);
8796
continue;
8897
}
8998

9099
Optional<Statement> lookupStatement = lookupCall.findAncestor(Statement.class);
91100
if (lookupStatement.isEmpty()) {
92-
UnfixedFinding unfixedFinding =
93-
new UnfixedFinding(
94-
findingId, detectorRule, path, line, "No statement found around lookup call");
95-
unfixedFindings.add(unfixedFinding);
101+
issues.stream()
102+
.map(getKey)
103+
.map(
104+
findingId ->
105+
new UnfixedFinding(
106+
findingId,
107+
detectorRule,
108+
path,
109+
line,
110+
"No statement found around lookup call"))
111+
.forEach(unfixedFindings::add);
96112
continue;
97113
}
98114

@@ -102,26 +118,47 @@ public <T> CodemodFileScanningResult remediateAll(
102118

103119
Optional<Node> lookupParentNode = lookupStatement.get().getParentNode();
104120
if (lookupParentNode.isEmpty()) {
105-
UnfixedFinding unfixedFinding =
106-
new UnfixedFinding(
107-
findingId, detectorRule, path, line, "No parent node found around lookup call");
108-
unfixedFindings.add(unfixedFinding);
121+
issues.stream()
122+
.map(getKey)
123+
.map(
124+
findingId ->
125+
new UnfixedFinding(
126+
findingId,
127+
detectorRule,
128+
path,
129+
line,
130+
"No parent node found around lookup call"))
131+
.forEach(unfixedFindings::add);
109132
continue;
110133
}
111134

112135
if (!(lookupParentNode.get() instanceof BlockStmt blockStmt)) {
113-
UnfixedFinding unfixedFinding =
114-
new UnfixedFinding(
115-
findingId, detectorRule, path, line, "No block statement found around lookup call");
116-
unfixedFindings.add(unfixedFinding);
136+
issues.stream()
137+
.map(getKey)
138+
.map(
139+
findingId ->
140+
new UnfixedFinding(
141+
findingId,
142+
detectorRule,
143+
path,
144+
line,
145+
"No block statement found around lookup call"))
146+
.forEach(unfixedFindings::add);
117147
continue;
118148
}
119149

120150
// add the validation call to the block statement
121151
int index = blockStmt.getStatements().indexOf(lookupStatement.get());
122152
List<DependencyGAV> deps =
123153
fixStrategy.fix(cu, parentClass, lookupCall, contextNameVariable, blockStmt, index);
124-
changes.add(CodemodChange.from(line, deps, new FixedFinding(findingId, detectorRule)));
154+
155+
List<FixedFinding> fixedFindings =
156+
issues.stream()
157+
.map(getKey)
158+
.map(findingId -> new FixedFinding(findingId, detectorRule))
159+
.toList();
160+
161+
changes.add(CodemodChange.from(line, deps, fixedFindings));
125162
}
126163

127164
List<UnfixedFinding> allUnfixedFindings = new ArrayList<>(results.unfixableFindings());

0 commit comments

Comments
 (0)