Skip to content

Commit c9fa81e

Browse files
carlosu7nahsra
carlosu7
and
nahsra
authored
Sonarcloud codemod - Harden String Parse to primitives (Integer/Float constructor cases) (#252)
Finish Sonarcloud codemod (HardenStringParseToPrimitivesCodemod) to enforce the appropriate parsing technique for converting Strings to primitive types in the codebase. This PR handles the `new Float`//`new Integer` constructor cases (ObjectCreationExpr nodes). Previous PR that handles `.valueOf` cases -> #250 Sonar rule reference https://rules.sonarsource.com/java/RSPEC-2130/ sonar cloud issues related to string-to-primitive conversion: https://sonarcloud.io/project/issues?fileUuids=AYvtrjqILCzGLicz7AY_&resolved=false&id=nahsra_WebGoat_10_23 **IMPORTANT NOTE!!!!** As you can see on the [Test.java.before](https://github.com/pixee/codemodder-java/compare/sonar-codemod-integer-parseInt...sonar-codemod-harden-string-parsing-CONSTRUCTORS?expand=1#diff-4676d84eb5c765c1163ce8dde3e2f8a0c437ef1122c9e6ad490ff599557a655b) the following cases are not reported by [sonarcloud](https://sonarcloud.io/project/issues?fileUuids=AYvtrjqILCzGLicz7AY_&resolved=false&id=nahsra_WebGoat_10_23): ``` float myFloatValue = (new Float(myNum)).floatValue(); int myIntValue = (new Integer(myNum)).intValue(); ``` Therefore, those cases are not being fixed --------- Co-authored-by: Arshan Dabirsiaghi <[email protected]>
1 parent 713ba46 commit c9fa81e

File tree

5 files changed

+152
-36
lines changed

5 files changed

+152
-36
lines changed

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

Lines changed: 77 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@
33
import com.github.javaparser.ast.CompilationUnit;
44
import com.github.javaparser.ast.Node;
55
import com.github.javaparser.ast.NodeList;
6-
import com.github.javaparser.ast.expr.Expression;
7-
import com.github.javaparser.ast.expr.MethodCallExpr;
6+
import com.github.javaparser.ast.expr.*;
87
import com.github.javaparser.ast.type.Type;
98
import io.codemodder.*;
109
import io.codemodder.providers.sonar.ProvidedSonarScan;
@@ -24,14 +23,83 @@
2423
executionPriority = CodemodExecutionPriority.HIGH)
2524
public final class HardenStringParseToPrimitivesCodemod extends CompositeJavaParserChanger {
2625

27-
// TODO create another static sonar java parser changer to handle constructor cases
28-
// (ObjectCreationExpr)
2926
@Inject
3027
public HardenStringParseToPrimitivesCodemod(
31-
final HardenParseForValueOfChanger parseMethodCallExprChanger) {
32-
super(parseMethodCallExprChanger);
28+
final HardenParseForConstructorChanger hardenParseForConstructorChanger,
29+
final HardenParseForValueOfChanger hardenParseForValueOfChanger) {
30+
super(hardenParseForConstructorChanger, hardenParseForValueOfChanger);
3331
}
3432

33+
private static Optional<String> determineParsingMethodForType(final String type) {
34+
if ("java.lang.Integer".equals(type) || "Integer".equals(type)) {
35+
return Optional.of("parseInt");
36+
}
37+
38+
if ("java.lang.Float".equals(type) || "Float".equals(type)) {
39+
return Optional.of("parseFloat");
40+
}
41+
42+
return Optional.empty();
43+
}
44+
45+
/**
46+
* Handles cases where Strings are converted to numbers using constructors like new Integer("24")
47+
* or new Float("12").
48+
*/
49+
private static final class HardenParseForConstructorChanger
50+
extends SonarPluginJavaParserChanger<ObjectCreationExpr> {
51+
52+
@Inject
53+
public HardenParseForConstructorChanger(
54+
@ProvidedSonarScan(ruleId = "java:S2130") final RuleIssues issues) {
55+
super(
56+
issues,
57+
ObjectCreationExpr.class,
58+
RegionNodeMatcher.MATCHES_START,
59+
CodemodReporterStrategy.empty());
60+
}
61+
62+
@Override
63+
public boolean onIssueFound(
64+
final CodemodInvocationContext context,
65+
final CompilationUnit cu,
66+
final ObjectCreationExpr objectCreationExpr,
67+
final Issue issue) {
68+
69+
final String type = objectCreationExpr.getType().asString();
70+
final Expression argumentExpression = objectCreationExpr.getArguments().get(0);
71+
72+
final Optional<Expression> argument = extractArgumentExpression(argumentExpression);
73+
74+
final Optional<String> replacementMethod = determineParsingMethodForType(type);
75+
76+
if (replacementMethod.isPresent() && argument.isPresent()) {
77+
MethodCallExpr replacementExpr =
78+
new MethodCallExpr(new NameExpr(type), replacementMethod.get());
79+
80+
replacementExpr.addArgument(argument.get());
81+
82+
objectCreationExpr.replace(replacementExpr);
83+
return true;
84+
}
85+
86+
return false;
87+
}
88+
89+
private Optional<Expression> extractArgumentExpression(Expression argumentExpression) {
90+
if (argumentExpression instanceof StringLiteralExpr
91+
|| argumentExpression instanceof NameExpr) {
92+
return Optional.of(argumentExpression);
93+
}
94+
// Handle other cases or return null if unable to extract the argument expression
95+
return Optional.empty();
96+
}
97+
}
98+
99+
/**
100+
* Handles cases where Strings are converted to numbers using the static method .valueOf() from
101+
* Integer or Float classes.
102+
*/
35103
private static final class HardenParseForValueOfChanger
36104
extends SonarPluginJavaParserChanger<MethodCallExpr> {
37105

@@ -57,10 +125,10 @@ public boolean onIssueFound(
57125
if ("valueOf".equals(methodName)) {
58126
final String targetType = retrieveTargetTypeFromMethodCallExpr(methodCallExpr);
59127

60-
final String replacementMethod = determineParsingMethodForType(targetType);
128+
final Optional<String> replacementMethod = determineParsingMethodForType(targetType);
61129

62-
if (replacementMethod != null) {
63-
methodCallExpr.setName(replacementMethod);
130+
if (replacementMethod.isPresent()) {
131+
methodCallExpr.setName(replacementMethod.get());
64132

65133
return handleMethodCallChainsAfterValueOfIfNeeded(methodCallExpr);
66134
}
@@ -79,15 +147,6 @@ private String retrieveTargetTypeFromMethodCallExpr(final MethodCallExpr methodC
79147
: optionalTypeArguments.get().get(0).toString();
80148
}
81149

82-
private String determineParsingMethodForType(final String type) {
83-
return switch (type) {
84-
case "java.lang.Integer" -> "parseInt";
85-
case "java.lang.Float" -> "parseFloat";
86-
// Add more cases if needed
87-
default -> null;
88-
};
89-
}
90-
91150
private boolean handleMethodCallChainsAfterValueOfIfNeeded(
92151
final MethodCallExpr methodCallExpr) {
93152

‎core-codemods/src/test/resources/harden-string-parse-to-primitives-s2130/Test.java.after‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,19 @@ public class Flags {
1717

1818
public Flag getFlag(Lesson forLesson) {
1919
String myNum = "42.0";
20-
float myFloat = new Float(myNum);
20+
//comment
2121

2222
String lessonName = forLesson.getName();
2323
int challengeNumber = Integer.parseInt(lessonName.substring(lessonName.length() - 1));
24-
float floatNumber = Float.parseFloat(myNum);
24+
float floatNumber = Float.parseFloat("12.1");
2525
float floatValue = Float.parseFloat(myNum);
26-
float intValue = Integer.parseInt(myNum);
27-
System.out.println(myFloat + floatValue + intValue + floatNumber);
26+
float intValue = Integer.parseInt("12.1");
27+
28+
float myFloat = Float.parseFloat(myNum);
29+
int myInteger = Integer.parseInt("42.0");
30+
float myFloatValue = (new Float(myNum)).floatValue();
31+
int myIntValue = (new Integer(myNum)).intValue();
32+
System.out.println(myFloat + floatValue + intValue + floatNumber + myInteger + myIntValue + myFloatValue + myFloat);
2833
return FLAGS.get(challengeNumber);
2934
}
3035

‎core-codemods/src/test/resources/harden-string-parse-to-primitives-s2130/Test.java.before‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,19 @@ public class Flags {
1717

1818
public Flag getFlag(Lesson forLesson) {
1919
String myNum = "42.0";
20-
float myFloat = new Float(myNum);
20+
//comment
2121

2222
String lessonName = forLesson.getName();
2323
int challengeNumber = Integer.valueOf(lessonName.substring(lessonName.length() - 1));
24-
float floatNumber = Float.valueOf(myNum);
24+
float floatNumber = Float.valueOf("12.1");
2525
float floatValue = Float.valueOf(myNum).floatValue();
26-
float intValue = Integer.valueOf(myNum).intValue();
27-
System.out.println(myFloat + floatValue + intValue + floatNumber);
26+
float intValue = Integer.valueOf("12.1").intValue();
27+
28+
float myFloat = new Float(myNum);
29+
int myInteger = new Integer("42.0");
30+
float myFloatValue = (new Float(myNum)).floatValue();
31+
int myIntValue = (new Integer(myNum)).intValue();
32+
System.out.println(myFloat + floatValue + intValue + floatNumber + myInteger + myIntValue + myFloatValue + myFloat);
2833
return FLAGS.get(challengeNumber);
2934
}
3035

‎core-codemods/src/test/resources/harden-string-parse-to-primitives-s2130/sonar-issues.json‎

Lines changed: 45 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,50 @@
11
{
2-
"total": 5,
2+
"total": 6,
33
"p": 1,
44
"ps": 500,
55
"paging": {
66
"pageIndex": 1,
77
"pageSize": 500,
8-
"total": 5
8+
"total": 6
99
},
10-
"effortTotal": 25,
11-
"debtTotal": 25,
10+
"effortTotal": 30,
11+
"debtTotal": 30,
1212
"issues": [
13+
{
14+
"key": "AYwmb6JsSgLxngfUhaWn",
15+
"rule": "java:S2130",
16+
"severity": "MINOR",
17+
"component": "nahsra_WebGoat_10_23:src/main/java/org/owasp/webgoat/lessons/challenges/Flags.java",
18+
"project": "nahsra_WebGoat_10_23",
19+
"line": 29,
20+
"hash": "e8d98f9baed7eca69d81e26f0dbf17bc",
21+
"textRange": {
22+
"startLine": 29,
23+
"endLine": 29,
24+
"startOffset": 22,
25+
"endOffset": 40
26+
},
27+
"flows": [],
28+
"status": "OPEN",
29+
"message": "Use \"Integer.parseInt\" for this string-to-int conversion.",
30+
"effort": "5min",
31+
"debt": "5min",
32+
"tags": [
33+
"performance"
34+
],
35+
"creationDate": "2023-12-01T18:28:54+0100",
36+
"updateDate": "2023-12-01T18:28:54+0100",
37+
"type": "CODE_SMELL",
38+
"organization": "nahsra",
39+
"cleanCodeAttribute": "CONVENTIONAL",
40+
"cleanCodeAttributeCategory": "CONSISTENT",
41+
"impacts": [
42+
{
43+
"softwareQuality": "MAINTAINABILITY",
44+
"severity": "LOW"
45+
}
46+
]
47+
},
1348
{
1449
"key": "AYwihbvytTDRswqzoUqo",
1550
"rule": "java:S2130",
@@ -121,13 +156,13 @@
121156
"severity": "MINOR",
122157
"component": "nahsra_WebGoat_10_23:src/main/java/org/owasp/webgoat/lessons/challenges/Flags.java",
123158
"project": "nahsra_WebGoat_10_23",
124-
"line": 20,
125-
"hash": "f6070e2b27ea25b14551cb2e887631a2",
159+
"line": 28,
160+
"hash": "bd129f84f059826048f35d0736374bfa",
126161
"textRange": {
127-
"startLine": 20,
128-
"endLine": 20,
129-
"startOffset": 20,
130-
"endOffset": 36
162+
"startLine": 28,
163+
"endLine": 28,
164+
"startOffset": 22,
165+
"endOffset": 38
131166
},
132167
"flows": [],
133168
"status": "OPEN",

‎plugins/codemodder-plugin-sonar/src/main/java/io/codemodder/providers/sonar/SonarModule.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,9 @@ protected void configure() {
8282
.flatMap(Arrays::stream)
8383
.toList();
8484

85+
// Outside the loop, create a map to track already bound ruleIds
86+
final Map<String, Boolean> boundRuleIds = new HashMap<>();
87+
8588
injectableParams.forEach(
8689
param -> {
8790
ProvidedSonarScan annotation = param.getAnnotation(ProvidedSonarScan.class);
@@ -95,6 +98,13 @@ protected void configure() {
9598
+ ")");
9699
}
97100

101+
final String ruleId = annotation.ruleId();
102+
103+
// Check if the ruleId has already been bound
104+
if (boundRuleIds.containsKey(ruleId)) {
105+
return; // Skip binding if already bound
106+
}
107+
98108
// bind from existing scan
99109
List<Issue> issues = perRuleBreakdown.get(annotation.ruleId());
100110

@@ -114,6 +124,8 @@ protected void configure() {
114124
});
115125
RuleIssues ruleIssues = new DefaultRuleIssues(issuesByPath);
116126
bind(RuleIssues.class).annotatedWith(annotation).toInstance(ruleIssues);
127+
// Mark the ruleId as bound
128+
boundRuleIds.put(ruleId, true);
117129
}
118130
});
119131
}

0 commit comments

Comments
 (0)