paulmillr / es6-shim

ECMAScript 6 compatibility shims for legacy JS engines
http://paulmillr.com
MIT License
3.11k stars 387 forks source link

Fix for bad hex strings when using `Number` constructor. #369

Closed Xotic750 closed 8 years ago

Xotic750 commented 8 years ago

Ref https://github.com/paulmillr/es6-shim/pull/366

Node v0.8, old Chrome/Chromium and others.

ljharb commented 8 years ago

Can you please provide tests that would fail without your change?

Also, can you link to spec text that says that +'-0x1' should be NaN, when the literal -0x1 parses fine?

Xotic750 commented 8 years ago

I already provided the tests in https://github.com/paulmillr/es6-shim/pull/366 and that was the only failing test remaining in your latest push. If I can find something concrete in the spec I will link it, but otherwise Node v0.8, old Chrome and Chromium (possibly others) behaviour is different to all others in this respect. So someone's got it right and someone's got it wrong, not consistent.

ljharb commented 8 years ago

You're right about the tests - https://travis-ci.org/paulmillr/es6-shim/jobs/89172994 passes but https://travis-ci.org/paulmillr/es6-shim/jobs/88743727 fails.

It's worth noting that this appears to make things consistent, but I do want to know what the spec requires.

Xotic750 commented 8 years ago

i would guess these http://www.ecma-international.org/ecma-262/6.0/#sec-tonumber-applied-to-the-string-type

StrNumericLiteral ::: StrDecimalLiteral BinaryIntegerLiteral OctalIntegerLiteral HexIntegerLiteral

http://www.ecma-international.org/ecma-262/6.0/#sec-literals-numeric-literals

HexIntegerLiteral :: 0x HexDigits 0X HexDigits HexDigits :: HexDigit HexDigits HexDigit HexDigit :: one of 0 1 2 3 4 5 6 7 8 9 a b c d e f A B C D E F

http://www.ecma-international.org/ecma-262/6.0/#sec-number-conversions

HexIntegerLiteral ::: 0x HexDigit 0X HexDigit HexIntegerLiteral HexDigit

So, current environments seem to interpret this as '0x1' is valid but '-0x1' is invalid. There is no mention of +/-, so it seems fair to interpret it this way I think. In the same way binary and octal strings do not mention +/- and they also are interpreted '0b1' is valid and '-0b1' is invalid. :/

And of course native parseInt behaves differently again by accepting '-0x1' but not '-0b1', but the es5-shim doesn't appear to agree in its fix. :/

// ES-5 15.1.2.2
if (parseInt(ws + '08') !== 8 || parseInt(ws + '0x16') !== 22) {
    /* global parseInt: true */
    parseInt = (function (origParseInt) {
        var hexRegex = /^0[xX]/;
        return function parseInt(str, radix) {
            var string = $String(str).trim();
            var defaultedRadix = $Number(radix) || (hexRegex.test(string) ? 16 : 10);
            return origParseInt(string, defaultedRadix);
        };
    }(parseInt));
}

That's about as concrete as I can give you.

ljharb commented 8 years ago

Please file a separate issue for parseInt