Skip to content

Commit

Permalink
Make ToNumber(String) conversion more spec-compliant
Browse files Browse the repository at this point in the history
Update ToNumber conversion applied to the String type:
* plus (+) or minus (-) signs are not allowed before the hex literals,
  return NaN for them
* also return NaN for HexIntegralLiteral followed by anything other than
  whitespace (in contrast to how 'parseInt()' works)
* add support for binary ('0b101010') and octal ('0o52') literals

New behavior is applied only if the language version is >=ES6, so it
shouldn't brake any existing code.

This also enables built-ins/Number test262 cases.
  • Loading branch information
sainaen committed Jan 25, 2018
1 parent 230a542 commit 33614f1
Show file tree
Hide file tree
Showing 6 changed files with 417 additions and 36 deletions.
2 changes: 1 addition & 1 deletion src/org/mozilla/javascript/NativeGlobal.java
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ static Object js_parseInt(Object[] args) {
}
}

double d = ScriptRuntime.stringToNumber(s, start, radix);
double d = ScriptRuntime.stringPrefixToNumber(s, start, radix);
return ScriptRuntime.wrapNumber(negative ? -d : d);
}

Expand Down
110 changes: 76 additions & 34 deletions src/org/mozilla/javascript/ScriptRuntime.java
Original file line number Diff line number Diff line change
Expand Up @@ -462,14 +462,21 @@ public static double toNumber(Object[] args, int index) {

public static final Double NaNobj = new Double(NaN);

static double stringPrefixToNumber(String s, int start, int radix) {
return stringToNumber(s, start, s.length() - 1, radix, true);
}

static double stringToNumber(String s, int start, int end, int radix) {
return stringToNumber(s, start, end, radix, false);
}

/*
* Helper function for toNumber, parseInt, and TokenStream.getToken.
*/
static double stringToNumber(String s, int start, int radix) {
private static double stringToNumber(String source, int sourceStart, int sourceEnd, int radix, boolean isPrefix) {
char digitMax = '9';
char lowerCaseBound = 'a';
char upperCaseBound = 'A';
int len = s.length();
if (radix < 10) {
digitMax = (char) ('0' + radix - 1);
}
Expand All @@ -479,20 +486,22 @@ static double stringToNumber(String s, int start, int radix) {
}
int end;
double sum = 0.0;
for (end=start; end < len; end++) {
char c = s.charAt(end);
for (end = sourceStart; end <= sourceEnd; end++) {
char c = source.charAt(end);
int newDigit;
if ('0' <= c && c <= digitMax)
newDigit = c - '0';
else if ('a' <= c && c < lowerCaseBound)
newDigit = c - 'a' + 10;
else if ('A' <= c && c < upperCaseBound)
newDigit = c - 'A' + 10;
else if (!isPrefix)
return NaN; // isn't a prefix but found unexpected char
else
break;
break; // unexpected char
sum = sum*radix + newDigit;
}
if (start == end) {
if (sourceStart == end) { // stopped right at the beginning
return NaN;
}
if (sum >= 9007199254740992.0) {
Expand All @@ -503,7 +512,7 @@ else if ('A' <= c && c < upperCaseBound)
* answer.
*/
try {
return Double.parseDouble(s.substring(start, end));
return Double.parseDouble(source.substring(sourceStart, end));
} catch (NumberFormatException nfe) {
return NaN;
}
Expand Down Expand Up @@ -535,12 +544,13 @@ else if ('A' <= c && c < upperCaseBound)
boolean bit53 = false;
// bit54 is the 54th bit (the first dropped from the mantissa)
boolean bit54 = false;
int pos = sourceStart;

for (;;) {
if (bitShiftInChar == 1) {
if (start == end)
if (pos == end)
break;
digit = s.charAt(start++);
digit = source.charAt(pos++);
if ('0' <= digit && digit <= '9')
digit -= '0';
else if ('a' <= digit && digit <= 'z')
Expand Down Expand Up @@ -614,61 +624,93 @@ else if ('a' <= digit && digit <= 'z')
return sum;
}


/**
* ToNumber applied to the String type
*
* See ECMA 9.3.1
* See the #sec-tonumber-applied-to-the-string-type section of ECMA
*/
public static double toNumber(String s) {
int len = s.length();
final int len = s.length();

// Skip whitespace at the start
int start = 0;
char startChar;
for (;;) {
if (start == len) {
// Empty or contains only whitespace
// empty or contains only whitespace
return +0.0;
}
startChar = s.charAt(start);
if (!ScriptRuntime.isStrWhiteSpaceChar(startChar))
if (!ScriptRuntime.isStrWhiteSpaceChar(startChar)) {
// found first non-whitespace character
break;
}
start++;
}

// Skip whitespace at the end
int end = len - 1;
char endChar;
while (ScriptRuntime.isStrWhiteSpaceChar(endChar = s.charAt(end))) {
end--;
}

// Do not break scripts relying on old non-compliant conversion
// (see bug #368)
// 1. makes ToNumber parse only a valid prefix in hex literals (similar to 'parseInt()')
// ToNumber('0x10 something') => 16
// 2. allows plus and minus signs for hexadecimal numbers
// ToNumber('-0x10') => -16
// 3. disables support for binary ('0b10') and octal ('0o13') literals
// ToNumber('0b1') => NaN
// ToNumber('0o5') => NaN
final Context cx = Context.getCurrentContext();
final boolean oldParsingMode =
cx == null || cx.getLanguageVersion() < Context.VERSION_ES6;

// Handle non-base10 numbers
if (startChar == '0') {
if (start + 2 < len) {
int c1 = s.charAt(start + 1);
if (c1 == 'x' || c1 == 'X') {
// A hexadecimal number
return stringToNumber(s, start + 2, 16);
if (start + 2 <= end) {
final char radixC = s.charAt(start + 1);
int radix = -1;
if (radixC == 'x' || radixC == 'X') {
radix = 16;
} else if (!oldParsingMode && (radixC == 'o' || radixC == 'O')) {
radix = 8;
} else if (!oldParsingMode && (radixC == 'b' || radixC == 'B')) {
radix = 2;
}
if (radix != -1) {
if (oldParsingMode) {
return stringPrefixToNumber(s, start + 2, radix);
}
return stringToNumber(s, start + 2, end, radix);
}
}
} else if (startChar == '+' || startChar == '-') {
if (start + 3 < len && s.charAt(start + 1) == '0') {
int c2 = s.charAt(start + 2);
if (c2 == 'x' || c2 == 'X') {
// A hexadecimal number with sign
double val = stringToNumber(s, start + 3, 16);
} else if (oldParsingMode && (startChar == '+' || startChar == '-')) {
// If in old parsing mode, check for a signed hexadecimal number
if (start + 3 <= end && s.charAt(start + 1) == '0') {
final char radixC = s.charAt(start + 2);
if (radixC == 'x' || radixC == 'X') {
double val = stringPrefixToNumber(s, start + 3, 16);
return startChar == '-' ? -val : val;
}
}
}

int end = len - 1;
char endChar;
while (ScriptRuntime.isStrWhiteSpaceChar(endChar = s.charAt(end)))
end--;
if (endChar == 'y') {
// check for "Infinity"
if (startChar == '+' || startChar == '-')
if (startChar == '+' || startChar == '-') {
start++;
if (start + 7 == end && s.regionMatches(start, "Infinity", 0, 8))
}
if (start + 7 == end && s.regionMatches(start, "Infinity", 0, 8)) {
return startChar == '-'
? Double.NEGATIVE_INFINITY
: Double.POSITIVE_INFINITY;
? Double.NEGATIVE_INFINITY
: Double.POSITIVE_INFINITY;
}
return NaN;
}
// A non-hexadecimal, non-infinity number:
// A base10, non-infinity number:
// just try a normal floating point conversion
String sub = s.substring(start, end+1);
// Quick test to check string contains only valid characters because
Expand Down
2 changes: 1 addition & 1 deletion src/org/mozilla/javascript/TokenStream.java
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,7 @@ final int getToken() throws IOException
return Token.ERROR;
}
} else {
dval = ScriptRuntime.stringToNumber(numString, 0, base);
dval = ScriptRuntime.stringPrefixToNumber(numString, 0, base);
}

this.number = dval;
Expand Down
159 changes: 159 additions & 0 deletions testsrc/org/mozilla/javascript/tests/ToNumberConversionsTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
package org.mozilla.javascript.tests;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.mozilla.javascript.Context;
import org.mozilla.javascript.Scriptable;

import java.util.ArrayList;
import java.util.Collection;
import java.util.List;

import static org.junit.Assert.assertTrue;

/**
* Test cases for ToNumber conversion applied to a String type.
*
* See the #sec-tonumber-applied-to-the-string-type section of ECMA262.
*
* This is only enabled with a language version >= ES6 (200).
*/
@RunWith(Parameterized.class)
public class ToNumberConversionsTest {
private static final int[] OPT_LEVELS = {-1, 0, 9};
private static final Object[][] TESTS = {
// order: expected result, source string
// (0) special
{"-Infinity", " -Infinity "},
{"+Infinity", " +Infinity "},

// (1) bin
{"4", " 0b100 "},
{"5", "0B0000101 "},
// bin, invalid
{"NaN", "-0b100"},
{"NaN", "+0b100"},
{"NaN", "0b100.1"},
{"NaN", "0b100e1"},
{"NaN", "0b100e-1"},
{"NaN", "0b100e+1"},
{"NaN", "0b2"},
{"NaN", "0b"},
{"NaN", "1b1"},

// (2) oct
{"8", " 0o10 "},
{"10", "0O000012 "},
// oct, invalid
{"NaN", "-0o10"},
{"NaN", "+0o10"},
{"NaN", "0o10.1"},
{"NaN", "0o10e1"},
{"NaN", "0o10e-1"},
{"NaN", "0o10e+1"},
{"NaN", "0o9"},
{"NaN", "1o9"},
{"NaN", "0o"},

// (3) dec
{"210", " 210 "},
{"210", "21e1 "},
{"-210", " -0021.00e+1 "},
{"210", " +02100.00e-1"},
// dec, invalid
{"NaN", " 210d2"},
{"NaN", " 210 d2"},

// (4) hex
{"210", " 0x00d2 "},
{"210", " 0x00D2 "},
{"210", " 0X00D2 "},
{"53985", "0xd2e1"}, // 'an exponent without sign' variant is a valid hex literal
// hex, invalid
{"NaN", "-0xd2"},
{"NaN", "+0xd2"},
{"NaN", "0xd2.00"},
{"NaN", "0xd2e+1"},
{"NaN", "0xd2e-1"},
{"NaN", "0xd2g"},
{"NaN", "0xd2 g"},
{"NaN", "0x7f.0x0.0x0.0x1"},
{"NaN", "0x"},
{"NaN", "1xd2"},
{"NaN", "+1xd2"},
{"NaN", "-0x"}
};
private static final String PRELUDE =
"function eq(a,b) {" +
"if (a != a) return b != b;" +
"return a == b;" +
"}\n";

@Parameterized.Parameters(name = "ToNumber(\"{1}\") == {0} (opt={2})")
public static Collection<Object[]> data() {
List<Object[]> cases = new ArrayList<>();

for (int optLevel : OPT_LEVELS) {
for (Object[] test : TESTS) {
cases.add(new Object[]{test[0], test[1], optLevel});
}
}

return cases;
}

@Parameterized.Parameter(0)
public String expected;
@Parameterized.Parameter(1)
public String source;
@Parameterized.Parameter(2)
public int optLevel;

@SuppressWarnings("ConstantConditions")
private boolean execute(Context cx, Scriptable scope, String script) {
return (Boolean) cx.evaluateString(scope, script, "inline", 1, null);
}

public Context cx;
public Scriptable scope;

@Before
public void setup() {
cx = Context.enter();
cx.setOptimizationLevel(optLevel);
cx.setLanguageVersion(Context.VERSION_ES6);
scope = cx.initSafeStandardObjects();
}

@After
public void tearDown() {
Context.exit();
}

@Test
public void test_NumberConstructor() {
String script = String.format("%seq(Number(\"%s\"), %s)", PRELUDE, source, expected);
assertTrue("Number('" + source + "') doesn't produce " + expected, execute(cx, scope, script));
}

@Test
public void test_coercion() {
String script = String.format("%seq(+(\"%s\"), %s)", PRELUDE, source, expected);
assertTrue("+('" + source + "') doesn't produce " + expected, execute(cx, scope, script));
}

@Test
public void test_isNaN() {
String script = String.format("%seq(isNaN(\"%s\"), isNaN(%s))", PRELUDE, source, expected);
assertTrue("isNaN('" + source + "') !== isNaN(" + expected + ")", execute(cx, scope, script));
}

@Test
public void test_isFinite() {
String script = String.format("%seq(isFinite(\"%s\"), isFinite(%s))", PRELUDE, source, expected);
assertTrue("isFinite('" + source + "') !== isFinite(" + expected + ")", execute(cx, scope, script));
}
}
Loading

0 comments on commit 33614f1

Please sign in to comment.