[java] Use static Patterns for regex-matching

[zodac]

Motivation and Context

There were several locations where Pattern objects were being used to perform regex matches. This is inefficient as the Pattern would be recompiled on each call. I've extracted these to constants in the appropriate class to ensure they are only compiled once. In one case there was a regex being used to check for digits, which I've replaced with a simple non-regex based function which should be less complex.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Refactoring (no changes to public API)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed. (tested with bazel test //java/... --test_size_filters=small)

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Potential Bug The matcher variable is now initialized outside the if statement but only assigned within the if block. This could lead to a NullPointerException if the matcher doesn't match the pattern. final Matcher matcher = VERSION_PATTERN.matcher(version); if (matcher.matches()) { Matcher Reassignment The matcher variable is reassigned in the condition of the else-if statement, which could make the code harder to understand and maintain. Consider using a separate matcher variable. } else if ((matcher = LINUX_DEVICE_MAPPING_WITH_PERMISSIONS.matcher(deviceMappingDefined)) .matches()) { Performance Concern The new isAllDigits method iterates through each character which could be inefficient for long strings. Consider using a regex or Character.isDigit with a loop that returns early. private static boolean isAllDigits(final String input) { for (int i = 0; i < input.length(); i++) { if (!Character.isDigit(input.charAt(i))) { return false; } } return !input.isEmpty(); }

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix variable name mismatch The variable name min has been renamed to minor in the updated code, but the assignment to current.minorVersion still uses the old variable name min. This will cause a compilation error. java/src/org/openqa/selenium/Platform.java [405-406] current.majorVersion = major; -current.minorVersion = min; +current.minorVersion = minor; Apply this suggestion Suggestion importance[1-10]: 10 __ Why: This suggestion fixes a critical issue where the variable name was changed from 'min' to 'minor' in the declaration but not in the assignment to current.minorVersion, which would cause a compilation error. This is a high-impact fix for a definite bug.	High
	Prevent potential IndexOutOfBoundsException The code attempts to access the first character of line without checking if the line is empty, which could lead to a StringIndexOutOfBoundsException. Add a check to ensure the line is not empty before accessing its characters. java/src/org/openqa/selenium/grid/commands/InfoCommand.java [149-151] -if (line.charAt(0) == '=') { +if (!line.isEmpty() && line.charAt(0) == '=') { formattedText.append("\n"); } Apply this suggestion Suggestion importance[1-10]: 8 __ Why: The suggestion adds a null check before accessing the first character of a string, preventing a potential StringIndexOutOfBoundsException if the line is empty. This is an important defensive programming practice that prevents runtime errors.	Medium
Learned best practice	Add null checks before accessing properties of parameters to prevent NullPointerExceptions The isAllDigits method doesn't check if the input string is null before accessing its properties, which could lead to a NullPointerException. Add a null check at the beginning of the method to ensure safer null handling. java/src/org/openqa/selenium/net/Urls.java [104-112] private static boolean isAllDigits(final String input) { + if (input == null) { + return false; + } + for (int i = 0; i < input.length(); i++) { if (!Character.isDigit(input.charAt(i))) { return false; } } return !input.isEmpty(); } Apply this suggestion Suggestion importance[1-10]: 6	Low
General	Optimize empty string check The method checks if the input is empty only after iterating through all characters. This is inefficient and could lead to unnecessary iterations. Move the empty check to the beginning of the method to handle this edge case first. java/src/org/openqa/selenium/net/Urls.java [104-112] private static boolean isAllDigits(final String input) { + if (input.isEmpty()) { + return false; + } + for (int i = 0; i < input.length(); i++) { if (!Character.isDigit(input.charAt(i))) { return false; } } - return !input.isEmpty(); + return true; } Apply this suggestion Suggestion importance[1-10]: 4 __ Why: The suggestion improves code efficiency by checking for empty strings at the beginning rather than after iteration. While this is a valid optimization, it has a moderate impact as the performance difference would be minimal in most cases.	Low
Update

[pujagani]

Why is this changed? Just curious

[zodac]

Similar to the other changes to make the Pattern instances static. The #replaceAll(String, String) method considered the first argument to be a regex, meaning it needs to be compiled for each call.

But since this call is just matching a literal dot, we can replace it with #replace(char, char) which will iterate through the characters in the String and replaces them.

[pujagani]

Any reason for modifying this?

[zodac]

Minor performance improvement, while I'm in the same class. The #startsWith(String) method can take a String of multiple characters, so it needs to search through the String. Whereas if you're only searching for a single character, charAt(int) directly accesses the char[] backing the String and returns the value in a single call.

Not a Pattern, so I can move it into another PR if you think that's appropriate.

[pujagani]

This change makes sense. But these are unrelated to changes made as part of the rest of this PR. Can we please separate this out into a separate PR?

[zodac]

That's fine, I'll create a separate PR for these changes.