Hey Vitaly and Jonathan,
> As mentioned offline, I would move the null check right before "return this".
@Vitaly: Thanks again. Adjusted.
@Jonathan: Thanks. Thought about that as well, but I'd probably rather go with
explaining the explicit nullcheck.
See the adjusted patch below. What do you think?
===== PATCH ======
diff --git a/src/java.base/share/classes/java/lang/String.java
b/src/java.base/share/classes/java/lang/String.java
--- a/src/java.base/share/classes/java/lang/String.java
+++ b/src/java.base/share/classes/java/lang/String.java
@@ -2177,11 +2177,13 @@
*/
public String replace(CharSequence target, CharSequence replacement) {
String tgtStr = target.toString();
- String replStr = replacement.toString();
int j = indexOf(tgtStr);
if (j < 0) {
+ // Explicit nullcheck of replacement to fulfill NPE contract in
early out
+ Objects.requireNonNull(replacement);
return this;
}
+ String replStr = replacement.toString();
int tgtLen = tgtStr.length();
int tgtLen1 = Math.max(tgtLen, 1);
int thisLen = length();
