New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Number.parseInt : add support for '0b' (binary) and '0o' (octal) notation #927

Closed
warren-bank opened this Issue Jun 5, 2017 · 12 comments

Comments

Projects
None yet
4 participants
@warren-bank

warren-bank commented Jun 5, 2017

example:

/**
 * http://www.ecma-international.org/ecma-262/6.0/#sec-number.parseint
 * https://github.com/tc39/ecma262/raw/master/spec.html#sec-number.parseint
 *
 * https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/parseInt
 *
 * according to the ES2015 spec:
 *   Number.parseInt === parseInt
 *
 * issues:
 * - Number.parseInt('0xff') === 255  // literal notation supported in ES5, parseInt supported in ES5
 * - Number.parseInt('0b11') === 0    // literal notation supported in ES6, parseInt not supported
 * - Number.parseInt('0o77') === 0    // literal notation supported in ES6, parseInt not supported
 *
 * workaround:
 * - detect notation, trim prefix, assign proper radix
 **/
Number.parseInt = function(val, radix){
  var regex = /^0([box])(.+)$/i
  if (typeof val !== 'string') val = val.toString()
  val.replace(regex, function(match, p1, p2){
    switch(p1){
      case 'b':
      case 'B':
        radix = 2; break
      case 'o':
      case 'O':
        radix = 8; break
      case 'x':
      case 'X':
        radix = 16; break
    }
    val = p2
  })
  return parseInt(val, radix)
}
@ljharb

This comment has been minimized.

Show comment
Hide comment
@ljharb

ljharb Jun 6, 2017

Member

Changing parseInt and Number.parseInt may not be web compatible.

What I think would be more useful is something like String.extractInt and String.extractFloat, which would result in a String that could be passed into Number().

Regardless, I think this should probably be discussed on es-discuss per https://github.com/tc39/ecma262/blob/master/CONTRIBUTING.md.

Member

ljharb commented Jun 6, 2017

Changing parseInt and Number.parseInt may not be web compatible.

What I think would be more useful is something like String.extractInt and String.extractFloat, which would result in a String that could be passed into Number().

Regardless, I think this should probably be discussed on es-discuss per https://github.com/tc39/ecma262/blob/master/CONTRIBUTING.md.

@ljharb ljharb closed this Jun 6, 2017

@bathos

This comment has been minimized.

Show comment
Hide comment
@bathos

bathos Jun 6, 2017

Contributor

fwiw, I think Number itself does provide functionality along these lines already. Given string input, it parses with the NumericLiteral production as its goal. It’s also sometimes preferable since it doesn’t accept strange input. Number.parseInt or Math.round etc can be called afterwards to ensure an int.

Number('0b10') // 2
Number('1cake') // NaN, unlike parseFloat() and parseInt(), which would give 1
Contributor

bathos commented Jun 6, 2017

fwiw, I think Number itself does provide functionality along these lines already. Given string input, it parses with the NumericLiteral production as its goal. It’s also sometimes preferable since it doesn’t accept strange input. Number.parseInt or Math.round etc can be called afterwards to ensure an int.

Number('0b10') // 2
Number('1cake') // NaN, unlike parseFloat() and parseInt(), which would give 1
@ljharb

This comment has been minimized.

Show comment
Hide comment
@ljharb

ljharb Jun 6, 2017

Member

@bathos that "accepting strange input" is the sole feature of parseFloat and parseInt that differentiates it from Number. It's always been the case that if you don't want that feature, you just use Number.

Member

ljharb commented Jun 6, 2017

@bathos that "accepting strange input" is the sole feature of parseFloat and parseInt that differentiates it from Number. It's always been the case that if you don't want that feature, you just use Number.

@rwaldron

This comment has been minimized.

Show comment
Hide comment
@rwaldron

rwaldron Jun 6, 2017

Contributor

Changing parseInt and Number.parseInt may not be web compatible.

It's absolutely not web compatible and the decision to not change them was very intentional. I proposed this change in 2014 https://github.com/rwaldron/tc39-notes/blob/master/es6/2014-04/apr-9.md#46-updates-to-parseint

Contributor

rwaldron commented Jun 6, 2017

Changing parseInt and Number.parseInt may not be web compatible.

It's absolutely not web compatible and the decision to not change them was very intentional. I proposed this change in 2014 https://github.com/rwaldron/tc39-notes/blob/master/es6/2014-04/apr-9.md#46-updates-to-parseint

@bathos

This comment has been minimized.

Show comment
Hide comment
@bathos

bathos Jun 6, 2017

Contributor

@ljharb yup, I was just mentioning it since it seemed pertinent that the functionality being requested is available with an existing native method (i.e., just pointing out to the OP that there’s a handy alternative already). Though I think in the case of parseInt the radix arg also differentiates its functionality?

Contributor

bathos commented Jun 6, 2017

@ljharb yup, I was just mentioning it since it seemed pertinent that the functionality being requested is available with an existing native method (i.e., just pointing out to the OP that there’s a handy alternative already). Though I think in the case of parseInt the radix arg also differentiates its functionality?

@warren-bank

This comment has been minimized.

Show comment
Hide comment
@warren-bank

warren-bank Jun 6, 2017

Sorry to bring up such an old topic.
I wasn't aware that this discussion goes back several years.
There's no doubt you've put much more thought into this.

With respect to web compatibility,
it seems that ES6 provides an opportunity for
Number.parseInt to diverge from parseInt,
and do things a bit better.

Similarly, Reflect.defineProperty
has a different return value from Object.defineProperty
..etc.

Just a quick note,
I noticed a bug in the example. (my bad)
The example ignores the input radix when it finds a pattern match.

In general,

  • 0b is a valid number having a radix >= 12
  • 0o is a valid number having a radix >= 25
  • 0x is a valid number having a radix >= 34

In consequence:

  • Number.parseInt('0b10', 16):
    • should equal:
      • Number.parseInt('0x0b10')
      • 2832
    • but results in:
      • Number.parseInt('10', 2)
      • 2

Simple fix:

  • if radix: do nothing

With respect to Number('0b10') ..parsed via NumericLiteral,

  • in order to parse this as hex (rather than binary),
    the prefix 0x needs to be prepended.

  • if the string value was supplied externally (ie: by user or data feed),
    then the software couldn't blindly add this prefix.
    it would need to detect and normalize the prefix, as provided.
    ex:

    • 0b10 => 0x0b10 works
    • 0x0b10 => 0x0x0b10 breaks
  • a better option might be:
    since:

    • 0x is not a valid number in hex
    • 0b is not a valid number in binary
    • 0o is not a valid number in octal

    a parser could detect the initial prefix and then trim any repeats

    • 0x0x0b10 => parseInt('0b10', 16)
    • 0x0b10 => parseInt('0b10', 16)
    • 0b10 => parseInt( '10', 2)

example:

Number.parseInt = function(val, radix){
  var regex = /^0([box])(?:0\1)*(.+)$/i
  if (radix === undefined){
    if (typeof val !== 'string') val = val.toString()
    val.replace(regex, function(match, p1, p2){
      switch(p1){
        case 'b':
        case 'B':
          radix = 2; break
        case 'o':
        case 'O':
          radix = 8; break
        case 'x':
        case 'X':
          radix = 16; break
      }
      val = p2
    })
  }
  return parseInt(val, radix)
}

For the sake of compatibility (and configurability),
maybe a 3rd optional parameter could be added,
to accept an options object?

By default, all options are false..
resulting in the same behavior as parseInt.
But:

Number.parseInt('0b10', undefined, {binary:true, octal:true, detectRepeats:true})

Just a thought..

warren-bank commented Jun 6, 2017

Sorry to bring up such an old topic.
I wasn't aware that this discussion goes back several years.
There's no doubt you've put much more thought into this.

With respect to web compatibility,
it seems that ES6 provides an opportunity for
Number.parseInt to diverge from parseInt,
and do things a bit better.

Similarly, Reflect.defineProperty
has a different return value from Object.defineProperty
..etc.

Just a quick note,
I noticed a bug in the example. (my bad)
The example ignores the input radix when it finds a pattern match.

In general,

  • 0b is a valid number having a radix >= 12
  • 0o is a valid number having a radix >= 25
  • 0x is a valid number having a radix >= 34

In consequence:

  • Number.parseInt('0b10', 16):
    • should equal:
      • Number.parseInt('0x0b10')
      • 2832
    • but results in:
      • Number.parseInt('10', 2)
      • 2

Simple fix:

  • if radix: do nothing

With respect to Number('0b10') ..parsed via NumericLiteral,

  • in order to parse this as hex (rather than binary),
    the prefix 0x needs to be prepended.

  • if the string value was supplied externally (ie: by user or data feed),
    then the software couldn't blindly add this prefix.
    it would need to detect and normalize the prefix, as provided.
    ex:

    • 0b10 => 0x0b10 works
    • 0x0b10 => 0x0x0b10 breaks
  • a better option might be:
    since:

    • 0x is not a valid number in hex
    • 0b is not a valid number in binary
    • 0o is not a valid number in octal

    a parser could detect the initial prefix and then trim any repeats

    • 0x0x0b10 => parseInt('0b10', 16)
    • 0x0b10 => parseInt('0b10', 16)
    • 0b10 => parseInt( '10', 2)

example:

Number.parseInt = function(val, radix){
  var regex = /^0([box])(?:0\1)*(.+)$/i
  if (radix === undefined){
    if (typeof val !== 'string') val = val.toString()
    val.replace(regex, function(match, p1, p2){
      switch(p1){
        case 'b':
        case 'B':
          radix = 2; break
        case 'o':
        case 'O':
          radix = 8; break
        case 'x':
        case 'X':
          radix = 16; break
      }
      val = p2
    })
  }
  return parseInt(val, radix)
}

For the sake of compatibility (and configurability),
maybe a 3rd optional parameter could be added,
to accept an options object?

By default, all options are false..
resulting in the same behavior as parseInt.
But:

Number.parseInt('0b10', undefined, {binary:true, octal:true, detectRepeats:true})

Just a thought..

@warren-bank

This comment has been minimized.

Show comment
Hide comment
@warren-bank

warren-bank Jun 6, 2017

oops..
just noticed something

parseInt('0x0b10',16) === parseInt('0x0b10') === parseInt('0b10',16)

by the same logic,
the "simple fix" in the example should also make the same allowance.

ie, the following should work:

  • Number.parseInt('0b10', 2)
  • Number.parseInt('0o10', 8)

example:

Number.parseInt = function(val, radix){
  var regex
  if (radix === undefined){
    regex = /^0([box])(?:0\1)*(.+)$/i
    if (typeof val !== 'string') val = val.toString()
    val.replace(regex, function(match, p1, p2){
      switch(p1){
        case 'b':
        case 'B':
          radix = 2; break
        case 'o':
        case 'O':
          radix = 8; break
        case 'x':
        case 'X':
          radix = 16; break
      }
      val = p2
    })
  }
  else {
    switch(radix){
      case 2:
        regex = /^(?:0b)+/i; break
      case 8:
        regex = /^(?:0o)+/i; break
      case 16:
        regex = /^(?:0x)+/i; break
    }
    if (regex){
      if (typeof val !== 'string') val = val.toString()
      val = val.replace(regex, '')
    }
  }
  return parseInt(val, radix)
}

warren-bank commented Jun 6, 2017

oops..
just noticed something

parseInt('0x0b10',16) === parseInt('0x0b10') === parseInt('0b10',16)

by the same logic,
the "simple fix" in the example should also make the same allowance.

ie, the following should work:

  • Number.parseInt('0b10', 2)
  • Number.parseInt('0o10', 8)

example:

Number.parseInt = function(val, radix){
  var regex
  if (radix === undefined){
    regex = /^0([box])(?:0\1)*(.+)$/i
    if (typeof val !== 'string') val = val.toString()
    val.replace(regex, function(match, p1, p2){
      switch(p1){
        case 'b':
        case 'B':
          radix = 2; break
        case 'o':
        case 'O':
          radix = 8; break
        case 'x':
        case 'X':
          radix = 16; break
      }
      val = p2
    })
  }
  else {
    switch(radix){
      case 2:
        regex = /^(?:0b)+/i; break
      case 8:
        regex = /^(?:0o)+/i; break
      case 16:
        regex = /^(?:0x)+/i; break
    }
    if (regex){
      if (typeof val !== 'string') val = val.toString()
      val = val.replace(regex, '')
    }
  }
  return parseInt(val, radix)
}
@rwaldron

This comment has been minimized.

Show comment
Hide comment
@rwaldron

rwaldron Jun 6, 2017

Contributor

I like the idea that @ljharb offered here: tc39/proposal-numeric-separator#11

Contributor

rwaldron commented Jun 6, 2017

I like the idea that @ljharb offered here: tc39/proposal-numeric-separator#11

@warren-bank

This comment has been minimized.

Show comment
Hide comment
@warren-bank

warren-bank Jun 6, 2017

another option is go a bit farther..

in the implementation of Number.parseInt,
to remove the "feature" of parseInt
that allows it to parse partial strings..
ie:

  • when a string begins with (one or more) valid characters
  • the string also contains invalid characters
  • the result is a Number that contains only the valid start of string

making Number.parseInt behave more like Number(...) in that it either succeeds or fails,
and in doing so we know with complete certainty whether or not the input string is valid.
..but adding support for a radix, and wrapping the result in Math.trunc.

taking the example a bit farther..
though probably not the most efficient way to go about doing this:

Number.parseInt = function(val, radix){
  var regex, result
  if (typeof val !== 'string') val = val.toString()
  if (radix === undefined){
    regex = /^0([box])(?:0\1)*(.+)$/i
    val.replace(regex, function(match, p1, p2){
      switch(p1){
        case 'b':
        case 'B':
          radix = 2; break
        case 'o':
        case 'O':
          radix = 8; break
        case 'x':
        case 'X':
          radix = 16; break
      }
      val = p2
    })
  }
  else {
    switch(radix){
      case 2:
        regex = /^(?:0b)+/i; break
      case 8:
        regex = /^(?:0o)+/i; break
      case 16:
        regex = /^(?:0x)+/i; break
    }
    if (regex){
      val = val.replace(regex, '')
    }
  }

  result = parseInt(val, radix)
  result = ((! Number.isNaN(result)) && (result.toString(radix) === val)) ? result : NaN

  return result
}
  • preprocess val and radix
  • get the result of parseInt
  • then round-trip by converting this result back to a string (in the proper radix)
  • verify that the 2 strings are exactly equal
  • if so, the result is completely correct
  • otherwise, the string contains an error: return NaN

warren-bank commented Jun 6, 2017

another option is go a bit farther..

in the implementation of Number.parseInt,
to remove the "feature" of parseInt
that allows it to parse partial strings..
ie:

  • when a string begins with (one or more) valid characters
  • the string also contains invalid characters
  • the result is a Number that contains only the valid start of string

making Number.parseInt behave more like Number(...) in that it either succeeds or fails,
and in doing so we know with complete certainty whether or not the input string is valid.
..but adding support for a radix, and wrapping the result in Math.trunc.

taking the example a bit farther..
though probably not the most efficient way to go about doing this:

Number.parseInt = function(val, radix){
  var regex, result
  if (typeof val !== 'string') val = val.toString()
  if (radix === undefined){
    regex = /^0([box])(?:0\1)*(.+)$/i
    val.replace(regex, function(match, p1, p2){
      switch(p1){
        case 'b':
        case 'B':
          radix = 2; break
        case 'o':
        case 'O':
          radix = 8; break
        case 'x':
        case 'X':
          radix = 16; break
      }
      val = p2
    })
  }
  else {
    switch(radix){
      case 2:
        regex = /^(?:0b)+/i; break
      case 8:
        regex = /^(?:0o)+/i; break
      case 16:
        regex = /^(?:0x)+/i; break
    }
    if (regex){
      val = val.replace(regex, '')
    }
  }

  result = parseInt(val, radix)
  result = ((! Number.isNaN(result)) && (result.toString(radix) === val)) ? result : NaN

  return result
}
  • preprocess val and radix
  • get the result of parseInt
  • then round-trip by converting this result back to a string (in the proper radix)
  • verify that the 2 strings are exactly equal
  • if so, the result is completely correct
  • otherwise, the string contains an error: return NaN
@bathos

This comment has been minimized.

Show comment
Hide comment
@bathos

bathos Jun 6, 2017

Contributor

@warren-bank I think generally https://esdiscuss.org/ is where you want to go with feature proposals, whereas this repo’s issues concern stuff like errors in the spec & editorial fixes etc.

Contributor

bathos commented Jun 6, 2017

@warren-bank I think generally https://esdiscuss.org/ is where you want to go with feature proposals, whereas this repo’s issues concern stuff like errors in the spec & editorial fixes etc.

@warren-bank

This comment has been minimized.

Show comment
Hide comment
@warren-bank

warren-bank Jun 6, 2017

I think I'm approaching this from the wrong angle..
a better question might be:

should the static (non-constructor) method Number() accept an optional radix?

ie:

  • Number('0123', 4) === parseInt('0123', 4) === 27

but:

  • Number('01239', 4) === NaN

whereas (for legacy reasons, i suppose):

  • Number.parseInt('01239', 4) === Number.parseInt('0123', 4) === 27

warren-bank commented Jun 6, 2017

I think I'm approaching this from the wrong angle..
a better question might be:

should the static (non-constructor) method Number() accept an optional radix?

ie:

  • Number('0123', 4) === parseInt('0123', 4) === 27

but:

  • Number('01239', 4) === NaN

whereas (for legacy reasons, i suppose):

  • Number.parseInt('01239', 4) === Number.parseInt('0123', 4) === 27
@warren-bank

This comment has been minimized.

Show comment
Hide comment
@warren-bank

warren-bank Jun 6, 2017

@bathos
I apologize if I brought this up in the wrong place.
Didn't mean to pollute the repo.

warren-bank commented Jun 6, 2017

@bathos
I apologize if I brought this up in the wrong place.
Didn't mean to pollute the repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment