Sonarcloud codemod - Harden String Parse to primitives (.valueOf cases) (#250)

Add new Sonarcloud codemod that enforces the appropriate parsing
technique for converting Strings to primitive types in the codebase.

This PR only handles the `.valueOf` cases (`MethodCallExpr` nodes), the
constructor cases (`ObjectCreationExpr` nodes) will be handled in
another PR .

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

---------

Co-authored-by: Arshan Dabirsiaghi <[email protected]>

Author: carlosu7

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

@@ -0,0 +1,113 @@
+package io.codemodder.codemods;
+
+import com.github.javaparser.ast.CompilationUnit;
+import com.github.javaparser.ast.Node;
+import com.github.javaparser.ast.NodeList;
+import com.github.javaparser.ast.expr.Expression;
+import com.github.javaparser.ast.expr.MethodCallExpr;
+import com.github.javaparser.ast.type.Type;
+import io.codemodder.*;
+import io.codemodder.providers.sonar.ProvidedSonarScan;
+import io.codemodder.providers.sonar.RuleIssues;
+import io.codemodder.providers.sonar.SonarPluginJavaParserChanger;
+import io.codemodder.providers.sonar.api.Issue;
+import java.util.Optional;
+import javax.inject.Inject;
+
+/**
+ * A codemod that enforces the appropriate parsing technique for converting Strings to primitive
+ * types in the codebase.
+ */
+@Codemod(
+    id = "sonar:java/harden-string-parse-to-primitives-s2130",
+    reviewGuidance = ReviewGuidance.MERGE_WITHOUT_REVIEW,
+    executionPriority = CodemodExecutionPriority.HIGH)
+public final class HardenStringParseToPrimitivesCodemod extends CompositeJavaParserChanger {
+
+  // TODO create another static sonar java parser changer to handle constructor cases
+  // (ObjectCreationExpr)
+  @Inject
+  public HardenStringParseToPrimitivesCodemod(
+      final HardenParseForValueOfChanger parseMethodCallExprChanger) {
+    super(parseMethodCallExprChanger);
+  }
+
+  private static final class HardenParseForValueOfChanger
+      extends SonarPluginJavaParserChanger<MethodCallExpr> {
+
+    @Inject
+    public HardenParseForValueOfChanger(
+        @ProvidedSonarScan(ruleId = "java:S2130") final RuleIssues issues) {
+      super(
+          issues,
+          MethodCallExpr.class,
+          RegionNodeMatcher.MATCHES_START,
+          CodemodReporterStrategy.empty());
+    }
+
+    @Override
+    public boolean onIssueFound(
+        final CodemodInvocationContext context,
+        final CompilationUnit cu,
+        final MethodCallExpr methodCallExpr,
+        final Issue issue) {
+
+      final String methodName = methodCallExpr.getNameAsString();
+
+      if ("valueOf".equals(methodName)) {
+        final String targetType = retrieveTargetTypeFromMethodCallExpr(methodCallExpr);
+
+        final String replacementMethod = determineParsingMethodForType(targetType);
+
+        if (replacementMethod != null) {
+          methodCallExpr.setName(replacementMethod);
+
+          return handleMethodCallChainsAfterValueOfIfNeeded(methodCallExpr);
+        }
+      }
+
+      return false;
+    }
+
+    private String retrieveTargetTypeFromMethodCallExpr(final MethodCallExpr methodCallExpr) {
+      final Optional<NodeList<Type>> optionalTypeArguments = methodCallExpr.getTypeArguments();
+      return optionalTypeArguments.isEmpty()
+          ? methodCallExpr
+              .getScope()
+              .map(expr -> expr.calculateResolvedType().describe())
+              .orElse("")
+          : optionalTypeArguments.get().get(0).toString();
+    }
+
+    private String determineParsingMethodForType(final String type) {
+      return switch (type) {
+        case "java.lang.Integer" -> "parseInt";
+        case "java.lang.Float" -> "parseFloat";
+          // Add more cases if needed
+        default -> null;
+      };
+    }
+
+    private boolean handleMethodCallChainsAfterValueOfIfNeeded(
+        final MethodCallExpr methodCallExpr) {
+
+      final Optional<Node> optionalParentNode = methodCallExpr.getParentNode();
+      if (optionalParentNode.isPresent()
+          && optionalParentNode.get() instanceof MethodCallExpr parentMethodCall) {
+
+        final String parentMethodName = parentMethodCall.getNameAsString();
+
+        final Optional<Expression> optionalScope = parentMethodCall.getScope();
+        if (optionalScope.isEmpty()) {
+          return false;
+        }
+
+        if ("intValue".equals(parentMethodName) || "floatValue".equals(parentMethodName)) {
+          parentMethodCall.replace(optionalScope.get());
+        }
+      }
+
+      return true;
+    }
+  }
+}

core-codemods/src/main/resources/io/codemodder/codemods/HardenStringParseToPrimitivesCodemod/description.md

@@ -0,0 +1,18 @@
+This change updates `String`-to-number conversions by leveraging the intended parse methods.
+
+This change makes developer intent clearer, and sometimes with a more concise expression.
+
+Our changes look like this:
+
+```diff
+    String number = "7.1";
+
+-   int integerNum = Integer.valueOf(number);
++   int integerNum = Integer.parseInt(number);
+
+-   float floatNumVal = Float.valueOf(number).floatValue();
++   float floatNumVal = Float.parseFloat(number);
+
+-   int integerNumber = new Integer(number);
++   int integerNumber = Integer.parseInt(number);
+```

core-codemods/src/main/resources/io/codemodder/codemods/HardenStringParseToPrimitivesCodemod/report.json

@@ -0,0 +1,7 @@
+{
+  "summary" : "Implemented parsing usage when converting Strings to primitives (Sonar).",
+  "change" : "Implemented parsing usage when converting Strings to primitives.",
+  "references" : [
+    "https://rules.sonarsource.com/java/RSPEC-2130/"
+  ]
+}

core-codemods/src/test/java/io/codemodder/codemods/HardenStringParseToPrimitivesCodemodTest.java

@@ -0,0 +1,11 @@
+package io.codemodder.codemods;
+
+import io.codemodder.testutils.CodemodTestMixin;
+import io.codemodder.testutils.Metadata;
+
+@Metadata(
+    codemodType = HardenStringParseToPrimitivesCodemod.class,
+    testResourceDir = "harden-string-parse-to-primitives-s2130",
+    renameTestFile = "src/main/java/org/owasp/webgoat/lessons/challenges/Flags.java",
+    dependencies = {})
+final class HardenStringParseToPrimitivesCodemodTest implements CodemodTestMixin {}

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

@@ -0,0 +1,34 @@
+package org.owasp.webgoat.lessons.challenges;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+import java.util.stream.IntStream;
+import org.owasp.webgoat.container.lessons.Lesson;
+import org.springframework.context.annotation.Configuration;
+
+@Configuration
+public class Flags {
+  private final Map<Integer, Flag> FLAGS = new HashMap<>();
+
+  public Flags() {
+    IntStream.range(1, 10).forEach(i -> FLAGS.put(i, new Flag(i, UUID.randomUUID().toString())));
+  }
+
+  public Flag getFlag(Lesson forLesson) {
+    String myNum = "42.0";
+    float myFloat = new Float(myNum);
+
+    String lessonName = forLesson.getName();
+    int challengeNumber = Integer.parseInt(lessonName.substring(lessonName.length() - 1));
+    float floatNumber = Float.parseFloat(myNum);
+    float floatValue = Float.parseFloat(myNum);
+    float intValue = Integer.parseInt(myNum);
+    System.out.println(myFloat + floatValue + intValue + floatNumber);
+    return FLAGS.get(challengeNumber);
+  }
+
+  public Flag getFlag(int flagNumber) {
+    return FLAGS.get(flagNumber);
+  }
+}

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

@@ -0,0 +1,34 @@
+package org.owasp.webgoat.lessons.challenges;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+import java.util.stream.IntStream;
+import org.owasp.webgoat.container.lessons.Lesson;
+import org.springframework.context.annotation.Configuration;
+
+@Configuration
+public class Flags {
+  private final Map<Integer, Flag> FLAGS = new HashMap<>();
+
+  public Flags() {
+    IntStream.range(1, 10).forEach(i -> FLAGS.put(i, new Flag(i, UUID.randomUUID().toString())));
+  }
+
+  public Flag getFlag(Lesson forLesson) {
+    String myNum = "42.0";
+    float myFloat = new Float(myNum);
+
+    String lessonName = forLesson.getName();
+    int challengeNumber = Integer.valueOf(lessonName.substring(lessonName.length() - 1));
+    float floatNumber = Float.valueOf(myNum);
+    float floatValue = Float.valueOf(myNum).floatValue();
+    float intValue = Integer.valueOf(myNum).intValue();
+    System.out.println(myFloat + floatValue + intValue + floatNumber);
+    return FLAGS.get(challengeNumber);
+  }
+
+  public Flag getFlag(int flagNumber) {
+    return FLAGS.get(flagNumber);
+  }
+}