Merge pull request #19115 from owen-mc/java/port/java/string-replace-all-with-non-regex

Java: Add new quality query to detect `String#replaceAll` with non-regex first argument

Author: owen-mc

java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected

@@ -9,3 +9,4 @@ ql/java/ql/src/Likely Bugs/Likely Typos/ContradictoryTypeChecks.ql
 ql/java/ql/src/Likely Bugs/Likely Typos/SuspiciousDateFormat.ql
 ql/java/ql/src/Likely Bugs/Resource Leaks/CloseReader.ql
 ql/java/ql/src/Likely Bugs/Resource Leaks/CloseWriter.ql
+ql/java/ql/src/Performance/StringReplaceAllWithNonRegex.ql

java/ql/src/Performance/StringReplaceAllWithNonRegex.md

@@ -0,0 +1,29 @@
+# Use of `String#replaceAll` with a first argument which is not a regular expression
+
+Using `String#replaceAll` is less performant than `String#replace` when the first argument is not a regular expression.
+
+## Overview
+
+The `String#replaceAll` method is designed to work with regular expressions as its first parameter. When you use a simple string without any regex patterns (like special characters or syntax), it's more efficient to use `String#replace` instead. This is because `replaceAll` has to compile the input as a regular expression first, which adds unnecessary overhead when you are just replacing literal text.
+
+## Recommendation
+
+Use `String#replace` instead where a `replaceAll` call uses a trivial string as its first argument.
+
+## Example
+
+```java
+public class Test {
+    void f() {
+        String s1 = "test";
+        s1 = s1.replaceAll("t", "x"); // NON_COMPLIANT
+        s1 = s1.replaceAll(".*", "x"); // COMPLIANT
+    }
+}
+
+```
+
+## References
+
+- Java SE Documentation: [String.replaceAll](https://docs.oracle.com/en/java/javase/20/docs/api/java.base/java/lang/String.html#replaceAll(java.lang.String,java.lang.String)).
+- Common Weakness Enumeration: [CWE-1176](https://cwe.mitre.org/data/definitions/1176.html).

java/ql/src/Performance/StringReplaceAllWithNonRegex.ql

@@ -0,0 +1,24 @@
+/**
+ * @id java/string-replace-all-with-non-regex
+ * @name Use of `String#replaceAll` with a first argument which is not a regular expression
+ * @description Using `String#replaceAll` with a first argument which is not a regular expression
+ *              is less efficient than using `String#replace`.
+ * @kind problem
+ * @precision very-high
+ * @problem.severity recommendation
+ * @tags quality
+ *       reliability
+ *       performance
+ *       external/cwe/cwe-1176
+ */
+
+import java
+
+from StringReplaceAllCall replaceAllCall, StringLiteral firstArg
+where
+  firstArg = replaceAllCall.getArgument(0) and
+  //only contains characters that could be a simple string
+  firstArg.getValue().regexpMatch("^[a-zA-Z0-9]+$")
+select replaceAllCall,
+  "This call to 'replaceAll' should be a call to 'replace' as its $@ is not a regular expression.",
+  firstArg, "first argument"

java/ql/src/codeql-suites/java-code-quality.qls

@@ -1,14 +1,15 @@
 - queries: .
 - include:
     id:
-      - java/suspicious-date-format
-      - java/integer-multiplication-cast-to-long
-      - java/equals-on-unrelated-types
       - java/contradictory-type-checks
-      - java/reference-equality-of-boxed-types
+      - java/equals-on-unrelated-types
       - java/inconsistent-equals-and-hashcode
-      - java/unchecked-cast-in-equals
-      - java/unused-container
       - java/input-resource-leak
+      - java/integer-multiplication-cast-to-long
       - java/output-resource-leak
-      - java/type-variable-hides-type
+      - java/reference-equality-of-boxed-types
+      - java/string-replace-all-with-non-regex
+      - java/suspicious-date-format
+      - java/type-variable-hides-type
+      - java/unchecked-cast-in-equals
+      - java/unused-container

java/ql/test/query-tests/StringReplaceAllWithNonRegex/StringReplaceAllWithNonRegex.expected

@@ -0,0 +1 @@
+| Test.java:4:14:4:36 | replaceAll(...) | This call to 'replaceAll' should be a call to 'replace' as its $@ is not a regular expression. | Test.java:4:28:4:30 | "t" | first argument |

java/ql/test/query-tests/StringReplaceAllWithNonRegex/StringReplaceAllWithNonRegex.qlref

@@ -0,0 +1,2 @@
+query: Performance/StringReplaceAllWithNonRegex.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql

java/ql/test/query-tests/StringReplaceAllWithNonRegex/Test.java

@@ -0,0 +1,7 @@
+public class Test {
+    void f() {
+        String s1 = "test";
+        s1 = s1.replaceAll("t", "x"); // $ Alert // NON_COMPLIANT
+        s1 = s1.replaceAll(".*", "x"); // COMPLIANT
+    }
+}