Add null checks to String methods.
Also did some general cleanup and
fixed String tests to not be optimized by the compiler.
Change-Id: I503aebb5b60764e74406e408985bf57437e88498
diff --git a/user/super/com/google/gwt/emul/java/lang/Character.java b/user/super/com/google/gwt/emul/java/lang/Character.java
index ca0c93d..61db5e1 100644
--- a/user/super/com/google/gwt/emul/java/lang/Character.java
+++ b/user/super/com/google/gwt/emul/java/lang/Character.java
@@ -230,7 +230,7 @@
* TODO: correct Unicode handling.
*/
public static boolean isDigit(char c) {
- return String.valueOf(c).nativeMatches(digitRegex());
+ return digitRegex().test(String.valueOf(c));
}
private static native NativeRegExp digitRegex() /*-{
@@ -245,7 +245,7 @@
* TODO: correct Unicode handling.
*/
public static boolean isLetter(char c) {
- return String.valueOf(c).nativeMatches(leterRegex());
+ return leterRegex().test(String.valueOf(c));
}
private static native NativeRegExp leterRegex() /*-{
@@ -256,7 +256,7 @@
* TODO: correct Unicode handling.
*/
public static boolean isLetterOrDigit(char c) {
- return String.valueOf(c).nativeMatches(leterOrDigitRegex());
+ return leterOrDigitRegex().test(String.valueOf(c));
}
private static native NativeRegExp leterOrDigitRegex() /*-{
@@ -296,11 +296,11 @@
}
public static boolean isWhitespace(char ch) {
- return String.valueOf(ch).nativeMatches(whitespaceRegex());
+ return whitespaceRegex().test(String.valueOf(ch));
}
public static boolean isWhitespace(int codePoint) {
- return String.fromCodePoint(codePoint).nativeMatches(whitespaceRegex());
+ return whitespaceRegex().test(String.fromCodePoint(codePoint));
}
// The regex would just be /\s/, but browsers handle non-breaking spaces inconsistently. Also,
diff --git a/user/super/com/google/gwt/emul/java/lang/String.java b/user/super/com/google/gwt/emul/java/lang/String.java
index 1e355e3..18bd4c0 100644
--- a/user/super/com/google/gwt/emul/java/lang/String.java
+++ b/user/super/com/google/gwt/emul/java/lang/String.java
@@ -210,13 +210,6 @@
return replaceStr;
}
- private static native int compareTo(String thisStr, String otherStr) /*-{
- if (thisStr == otherStr) {
- return 0;
- }
- return thisStr < otherStr ? -1 : 1;
- }-*/;
-
private static Charset getCharset(String charsetName) throws UnsupportedEncodingException {
try {
return Charset.forName(charsetName);
@@ -382,15 +375,15 @@
@Override
public int compareTo(String other) {
- return compareTo(this, other);
+ return JsUtils.compare(checkNotNull(this), checkNotNull(other));
}
public int compareToIgnoreCase(String other) {
- return compareTo(toLowerCase(), other.toLowerCase());
+ return toLowerCase().compareTo(other.toLowerCase());
}
public String concat(String str) {
- return this + str;
+ return checkNotNull(this) + checkNotNull(str);
}
public boolean contains(CharSequence s) {
@@ -421,18 +414,15 @@
}
public boolean equalsIgnoreCase(String other) {
- return equalsIgnoreCase(this, other);
- }
-
- private static native boolean equalsIgnoreCase(String s, String other) /*-{
+ checkNotNull(this);
if (other == null) {
return false;
}
- if (s == other) {
+ if (equals(other)) {
return true;
}
- return (s.length == other.length) && (s.toLowerCase() == other.toLowerCase());
- }-*/;
+ return length() == other.length() && toLowerCase().equals(other.toLowerCase());
+ }
public byte[] getBytes() {
// default character set for GWT is UTF-8
@@ -450,7 +440,10 @@
public void getChars(int srcBegin, int srcEnd, char[] dst, int dstBegin) {
checkCriticalStringBounds(srcBegin, srcEnd, length());
checkCriticalStringBounds(dstBegin, dstBegin + (srcEnd - srcBegin), dst.length);
+ getChars0(srcBegin, srcEnd, dst, dstBegin);
+ }
+ private void getChars0(int srcBegin, int srcEnd, char[] dst, int dstBegin) {
while (srcBegin < srcEnd) {
dst[dstBegin++] = charAt(srcBegin++);
}
@@ -476,9 +469,9 @@
public int indexOf(String str, int startIndex) {
return asNativeString().indexOf(str, startIndex);
}
-
+
public String intern() {
- return this;
+ return checkNotNull(this);
}
public boolean isEmpty() {
@@ -516,11 +509,7 @@
*/
public boolean matches(String regex) {
// We surround the regex with '^' and '$' because it must match the entire string.
- return nativeMatches(new NativeRegExp("^(" + regex + ")$"));
- }
-
- boolean nativeMatches(NativeRegExp regex) {
- return regex.test(this);
+ return new NativeRegExp("^(" + regex + ")$").test(this);
}
public int offsetByCodePoints(int index, int codePointOffset) {
@@ -529,13 +518,11 @@
public boolean regionMatches(boolean ignoreCase, int toffset, String other,
int ooffset, int len) {
- if (other == null) {
- throw new NullPointerException();
- }
+ checkNotNull(other);
if (toffset < 0 || ooffset < 0 || len <= 0) {
return false;
}
- if (toffset + len > this.length() || ooffset + len > other.length()) {
+ if (toffset + len > length() || ooffset + len > other.length()) {
return false;
}
@@ -692,11 +679,11 @@
@Override
public CharSequence subSequence(int beginIndex, int endIndex) {
- return this.substring(beginIndex, endIndex);
+ return substring(beginIndex, endIndex);
}
public String substring(int beginIndex) {
- return asNativeString().substr(beginIndex, this.length() - beginIndex);
+ return asNativeString().substr(beginIndex, length() - beginIndex);
}
public String substring(int beginIndex, int endIndex) {
@@ -704,9 +691,9 @@
}
public char[] toCharArray() {
- int n = this.length();
+ int n = length();
char[] charArr = new char[n];
- getChars(0, n, charArr, 0);
+ getChars0(0, n, charArr, 0);
return charArr;
}
@@ -833,15 +820,15 @@
}
protected static String $create(String other) {
- return other;
+ return checkNotNull(other);
}
protected static String $create(StringBuffer sb) {
- return String.valueOf(sb);
+ return sb.toString();
}
protected static String $create(StringBuilder sb) {
- return String.valueOf(sb);
+ return sb.toString();
}
@JsMethod
diff --git a/user/super/com/google/gwt/emul/java/lang/StringBuffer.java b/user/super/com/google/gwt/emul/java/lang/StringBuffer.java
index 39be45d..91f154a 100644
--- a/user/super/com/google/gwt/emul/java/lang/StringBuffer.java
+++ b/user/super/com/google/gwt/emul/java/lang/StringBuffer.java
@@ -1,12 +1,12 @@
/*
* Copyright 2008 Google Inc.
- *
+ *
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
- *
+ *
* http://www.apache.org/licenses/LICENSE-2.0
- *
+ *
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
@@ -15,9 +15,11 @@
*/
package java.lang;
+import static javaemul.internal.InternalPreconditions.checkNotNull;
+
/**
* A fast way to create strings using multiple appends.
- *
+ *
* This class is an exact clone of {@link StringBuilder} except for the name.
* Any change made to one should be mirrored in the other.
*/
@@ -28,7 +30,7 @@
}
public StringBuffer(CharSequence s) {
- super(String.valueOf(s));
+ super(s.toString());
}
/**
@@ -41,7 +43,7 @@
}
public StringBuffer(String s) {
- super(s);
+ super(checkNotNull(s));
}
public StringBuffer append(boolean x) {
diff --git a/user/super/com/google/gwt/emul/java/lang/StringBuilder.java b/user/super/com/google/gwt/emul/java/lang/StringBuilder.java
index 78bcd03..da705ef 100644
--- a/user/super/com/google/gwt/emul/java/lang/StringBuilder.java
+++ b/user/super/com/google/gwt/emul/java/lang/StringBuilder.java
@@ -1,12 +1,12 @@
/*
* Copyright 2008 Google Inc.
- *
+ *
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
- *
+ *
* http://www.apache.org/licenses/LICENSE-2.0
- *
+ *
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
@@ -15,9 +15,11 @@
*/
package java.lang;
+import static javaemul.internal.InternalPreconditions.checkNotNull;
+
/**
* A fast way to create strings using multiple appends.
- *
+ *
* This class is an exact clone of {@link StringBuffer} except for the name.
* Any change made to one should be mirrored in the other.
*/
@@ -28,7 +30,7 @@
}
public StringBuilder(CharSequence s) {
- super(String.valueOf(s));
+ super(s.toString());
}
/**
@@ -41,7 +43,7 @@
}
public StringBuilder(String s) {
- super(s);
+ super(checkNotNull(s));
}
public StringBuilder append(boolean x) {
diff --git a/user/super/com/google/gwt/emul/javaemul/internal/JsUtils.java b/user/super/com/google/gwt/emul/javaemul/internal/JsUtils.java
index 981f224..8796b61 100644
--- a/user/super/com/google/gwt/emul/javaemul/internal/JsUtils.java
+++ b/user/super/com/google/gwt/emul/javaemul/internal/JsUtils.java
@@ -20,6 +20,10 @@
*/
public class JsUtils {
+ public static native int compare(String a, String b) /*-{
+ return a == b ? 0 : (a < b ? -1 : 1);
+ }-*/;
+
public static native boolean isFinite(double d) /*-{
return isFinite(d);
}-*/;
diff --git a/user/test/com/google/gwt/emultest/java/lang/StringTest.java b/user/test/com/google/gwt/emultest/java/lang/StringTest.java
index b9f06aa..3b556ce 100644
--- a/user/test/com/google/gwt/emultest/java/lang/StringTest.java
+++ b/user/test/com/google/gwt/emultest/java/lang/StringTest.java
@@ -15,6 +15,7 @@
*/
package com.google.gwt.emultest.java.lang;
+import com.google.gwt.core.client.JavaScriptException;
import com.google.gwt.junit.client.GWTTestCase;
import com.google.gwt.testing.TestUtils;
@@ -31,22 +32,13 @@
*/
public class StringTest extends GWTTestCase {
- static volatile boolean TRUE = true;
-
- private static <T> T hideFromCompiler(T value) {
- return TRUE ? value : null;
- }
-
@Override
public String getModuleName() {
return "com.google.gwt.emultest.EmulSuite";
}
- /*
- * TODO: needs rewriting to avoid compiler optimizations.
- */
public void testCharAt() {
- assertEquals("abc".charAt(1), 'b');
+ assertEquals(hideFromCompiler("abc").charAt(1), 'b');
}
public void testCodePoint() {
@@ -103,6 +95,30 @@
assertEquals(1, nonBmpChar.codePointCount(0, 2));
}
+ public void testCompareTo() {
+ assertEquals(0, hideFromCompiler("a").compareTo("a"));
+ assertEquals(-1, hideFromCompiler("a").compareTo("b"));
+ assertEquals(1, hideFromCompiler("b").compareTo("a"));
+
+ try {
+ returnNull().compareTo("");
+ fail();
+ } catch (NullPointerException e) {
+ // expected
+ } catch (JavaScriptException e) {
+ // expected
+ }
+
+ try {
+ returnNull().compareTo(returnNull());
+ fail();
+ } catch (NullPointerException e) {
+ // expected
+ } catch (JavaScriptException e) {
+ // expected
+ }
+ }
+
public void testConcat() {
String abc = String.valueOf(new char[] {'a', 'b', 'c'});
String def = String.valueOf(new char[] {'d', 'e', 'f'});
@@ -118,6 +134,24 @@
assertEquals("abcd", s + c);
s += c;
assertEquals("abcd", s);
+
+ try {
+ returnNull().concat("");
+ fail();
+ } catch (NullPointerException e) {
+ // expected
+ } catch (JavaScriptException e) {
+ // expected
+ }
+
+ try {
+ hideFromCompiler("").concat(returnNull());
+ fail();
+ } catch (NullPointerException e) {
+ // expected
+ } catch (JavaScriptException e) {
+ // expected
+ }
}
public void testConstructor() {
@@ -147,6 +181,22 @@
sb = new StringBuilder();
sb.appendCodePoint(0x10400);
assertEquals("\uD801\uDC00", new String(sb));
+
+ try {
+ new String(returnNull());
+ } catch (NullPointerException e) {
+ // expected
+ } catch (JavaScriptException e) {
+ // expected
+ }
+
+ try {
+ new String(hideFromCompiler((StringBuilder) null));
+ } catch (NullPointerException e) {
+ // expected
+ } catch (JavaScriptException e) {
+ // expected
+ }
}
public void testConstructorBytes() {
@@ -265,26 +315,22 @@
}
}
- /*
- * TODO: needs rewriting to avoid compiler optimizations. (StringBuffer tests
- * are ok)
- */
public void testContains() {
// at the beginning
- assertTrue("abcdef".contains("ab"));
- assertTrue("abcdef".contains(new StringBuffer("ab")));
+ assertTrue(hideFromCompiler("abcdef").contains("ab"));
+ assertTrue(hideFromCompiler("abcdef").contains(new StringBuffer("ab")));
// at the end
- assertTrue("abcdef".contains("ef"));
- assertTrue("abcdef".contains(new StringBuffer("ef")));
+ assertTrue(hideFromCompiler("abcdef").contains("ef"));
+ assertTrue(hideFromCompiler("abcdef").contains(new StringBuffer("ef")));
// in the middle
- assertTrue("abcdef".contains("cd"));
- assertTrue("abcdef".contains(new StringBuffer("cd")));
+ assertTrue(hideFromCompiler("abcdef").contains("cd"));
+ assertTrue(hideFromCompiler("abcdef").contains(new StringBuffer("cd")));
// the same
- assertTrue("abcdef".contains("abcdef"));
- assertTrue("abcdef".contains(new StringBuffer("abcdef")));
+ assertTrue(hideFromCompiler("abcdef").contains("abcdef"));
+ assertTrue(hideFromCompiler("abcdef").contains(new StringBuffer("abcdef")));
// not present
- assertFalse("abcdef".contains("z"));
- assertFalse("abcdef".contains(new StringBuffer("z")));
+ assertFalse(hideFromCompiler("abcdef").contains("z"));
+ assertFalse(hideFromCompiler("abcdef").contains(new StringBuffer("z")));
}
public void testEndsWith() {
@@ -295,38 +341,64 @@
}
public void testEquals() {
- assertFalse("ABC".equals("abc"));
- assertFalse("abc".equals("ABC"));
- assertTrue("abc".equals("abc"));
- assertTrue("ABC".equals("ABC"));
- assertFalse("AbC".equals("aBC"));
- assertFalse("AbC".equals("aBC"));
- assertTrue("".equals(""));
- assertFalse("".equals(null));
+ assertFalse(hideFromCompiler("ABC").equals("abc"));
+ assertFalse(hideFromCompiler("abc").equals("ABC"));
+ assertTrue(hideFromCompiler("abc").equals("abc"));
+ assertTrue(hideFromCompiler("ABC").equals("ABC"));
+ assertFalse(hideFromCompiler("AbC").equals("aBC"));
+ assertFalse(hideFromCompiler("AbC").equals("aBC"));
+ assertTrue(hideFromCompiler("").equals(""));
+ assertFalse(hideFromCompiler("").equals(null));
- // Randomize the string to avoid static eval.
- double r = Math.random();
- assertTrue((r + "").equals(r + ""));
- assertFalse((r + "ABC").equals(r + "abc"));
- assertFalse((r + "abc").equals(r + "ABC"));
- assertTrue((r + "abc").equals(r + "abc"));
- assertTrue((r + "ABC").equals(r + "ABC"));
- assertFalse((r + "AbC").equals(r + "aBC"));
- assertFalse((r + "AbC").equals(r + "aBC"));
+ // TODO: String.equals does not have NPE check
+/*
+ try {
+ returnNull().equals("other");
+ fail();
+ } catch (NullPointerException e) {
+ // expected
+ } catch (JavaScriptException e) {
+ // expected
+ }
+
+ try {
+ returnNull().equals(returnNull());
+ fail();
+ } catch (NullPointerException e) {
+ // expected
+ } catch (JavaScriptException e) {
+ // expected
+ }
+*/
}
- /*
- * TODO: needs rewriting to avoid compiler optimizations.
- */
public void testEqualsIgnoreCase() {
- assertTrue("ABC".equalsIgnoreCase("abc"));
- assertTrue("abc".equalsIgnoreCase("ABC"));
- assertTrue("abc".equalsIgnoreCase("abc"));
- assertTrue("ABC".equalsIgnoreCase("ABC"));
- assertTrue("AbC".equalsIgnoreCase("aBC"));
- assertTrue("AbC".equalsIgnoreCase("aBC"));
- assertTrue("".equalsIgnoreCase(""));
- assertFalse("".equalsIgnoreCase(null));
+ assertTrue(hideFromCompiler("ABC").equalsIgnoreCase("abc"));
+ assertTrue(hideFromCompiler("abc").equalsIgnoreCase("ABC"));
+ assertTrue(hideFromCompiler("abc").equalsIgnoreCase("abc"));
+ assertTrue(hideFromCompiler("ABC").equalsIgnoreCase("ABC"));
+ assertTrue(hideFromCompiler("AbC").equalsIgnoreCase("aBC"));
+ assertTrue(hideFromCompiler("AbC").equalsIgnoreCase("aBC"));
+ assertTrue(hideFromCompiler("").equalsIgnoreCase(""));
+ assertFalse(hideFromCompiler("").equalsIgnoreCase(null));
+
+ try {
+ returnNull().equalsIgnoreCase("other");
+ fail();
+ } catch (NullPointerException e) {
+ // expected
+ } catch (JavaScriptException e) {
+ // expected
+ }
+
+ try {
+ returnNull().equalsIgnoreCase(returnNull());
+ fail();
+ } catch (NullPointerException e) {
+ // expected
+ } catch (JavaScriptException e) {
+ // expected
+ }
}
public void testGetBytesAscii() {
@@ -440,6 +512,15 @@
// get hashes again to verify the values are constant for a given string
assertEquals(expectedHash, testStrings[i].hashCode());
}
+
+ try {
+ returnNull().hashCode();
+ fail();
+ } catch (NullPointerException e) {
+ // expected
+ } catch (JavaScriptException e) {
+ // expected
+ }
}
public void testIndexOf() {
@@ -451,6 +532,15 @@
assertEquals(haystack.indexOf('a', 1), -1);
assertEquals(haystack.indexOf("bc"), 1);
assertEquals(haystack.indexOf(""), 0);
+
+ try {
+ returnNull().indexOf("");
+ fail();
+ } catch (NullPointerException e) {
+ // expected
+ } catch (JavaScriptException e) {
+ // expected
+ }
}
public void testIntern() {
@@ -458,6 +548,15 @@
String s2 = String.valueOf(new char[] {'a', 'b', 'c', 'd', 'e', 'f'});
assertTrue("strings not equal", s1.equals(s2));
assertSame("interns are not the same reference", s1.intern(), s2.intern());
+
+ try {
+ returnNull().intern();
+ fail();
+ } catch (NullPointerException e) {
+ // expected
+ } catch (JavaScriptException e) {
+ // expected
+ }
}
public void testLastIndexOf() {
@@ -479,21 +578,18 @@
assertEquals(4, cat.length());
}
- /*
- * TODO: needs rewriting to avoid compiler optimizations.
- */
public void testLowerCase() {
- assertEquals("abc", "AbC".toLowerCase());
- assertEquals("abc", "abc".toLowerCase());
- assertEquals("", "".toLowerCase());
+ assertEquals("abc", hideFromCompiler("AbC").toLowerCase());
+ assertEquals("abc", hideFromCompiler("abc").toLowerCase());
+ assertEquals("", hideFromCompiler("").toLowerCase());
- assertEquals("abc", "AbC".toLowerCase(Locale.US));
- assertEquals("abc", "abc".toLowerCase(Locale.US));
- assertEquals("", "".toLowerCase(Locale.US));
+ assertEquals("abc", hideFromCompiler("AbC").toLowerCase(Locale.US));
+ assertEquals("abc", hideFromCompiler("abc").toLowerCase(Locale.US));
+ assertEquals("", hideFromCompiler("").toLowerCase(Locale.US));
- assertEquals("abc", "AbC".toLowerCase(Locale.getDefault()));
- assertEquals("abc", "abc".toLowerCase(Locale.getDefault()));
- assertEquals("", "".toLowerCase(Locale.getDefault()));
+ assertEquals("abc", hideFromCompiler("AbC").toLowerCase(Locale.getDefault()));
+ assertEquals("abc", hideFromCompiler("abc").toLowerCase(Locale.getDefault()));
+ assertEquals("", hideFromCompiler("").toLowerCase(Locale.getDefault()));
}
public void testMatch() {
@@ -687,14 +783,10 @@
assertFalse(haystack.startsWith(haystack + "j"));
}
- /*
- * TODO: needs rewriting to avoid compiler optimizations.
- */
public void testSubstring() {
- String haystack = "abcdefghi";
- assertEquals("cd", haystack.substring(2, 4));
- assertEquals("bc", "abcdef".substring(1, 3));
- assertEquals("bcdef", "abcdef".substring(1));
+ assertEquals("cd", hideFromCompiler("abcdefghi").substring(2, 4));
+ assertEquals("bc", hideFromCompiler("abcdef").substring(1, 3));
+ assertEquals("bcdef", hideFromCompiler("abcdef").substring(1));
}
public void testToCharArray() {
@@ -712,7 +804,7 @@
assertEquals("concat with str.toString()", "0abcd", "0" + str.toString() + "d");
// issue 4301
- Object s = TRUE ? "abc" : createJavaScriptObject();
+ Object s = hideFromCompiler("abc");
assertSame("s same as s.toString()", s, s.toString());
}
@@ -751,21 +843,18 @@
trimRightAssertEquals("\u2029abc\u00a0","\u2029abc\u00a0");
}
- /*
- * TODO: needs rewriting to avoid compiler optimizations.
- */
public void testUpperCase() {
- assertEquals("ABC", "AbC".toUpperCase());
- assertEquals("ABC", "abc".toUpperCase());
- assertEquals("", "".toUpperCase());
+ assertEquals("ABC", hideFromCompiler("AbC").toUpperCase());
+ assertEquals("ABC", hideFromCompiler("abc").toUpperCase());
+ assertEquals("", hideFromCompiler("").toUpperCase());
- assertEquals("ABC", "AbC".toUpperCase(Locale.US));
- assertEquals("ABC", "abc".toUpperCase(Locale.US));
- assertEquals("", "".toUpperCase(Locale.US));
+ assertEquals("ABC", hideFromCompiler("AbC").toUpperCase(Locale.US));
+ assertEquals("ABC", hideFromCompiler("abc").toUpperCase(Locale.US));
+ assertEquals("", hideFromCompiler("").toUpperCase(Locale.US));
- assertEquals("ABC", "AbC".toUpperCase(Locale.getDefault()));
- assertEquals("ABC", "abc".toUpperCase(Locale.getDefault()));
- assertEquals("", "".toUpperCase(Locale.getDefault()));
+ assertEquals("ABC", hideFromCompiler("AbC").toUpperCase(Locale.getDefault()));
+ assertEquals("ABC", hideFromCompiler("abc").toUpperCase(Locale.getDefault()));
+ assertEquals("", hideFromCompiler("").toUpperCase(Locale.getDefault()));
}
/*
@@ -811,28 +900,20 @@
}
}
- private String returnNull() {
+ private <T> T hideFromCompiler(T value) {
if (Math.random() < -1) {
// Can never happen, but fools the compiler enough not to optimize this call.
fail();
- return "";
}
- return null;
+ return value;
+ }
+
+ private String returnNull() {
+ return hideFromCompiler(null);
}
private String toS(char from) {
return Character.toString(from);
}
- private static Object createJavaScriptObject() {
- if (TestUtils.isJvm()) {
- return new Object();
- }
- return createJavaScriptObject0();
- }
-
- private static native Object createJavaScriptObject0() /*-{
- return {};
- }-*/;
-
}
