Int.fromString unexpected behavior

[reebalazs]

I am testing on Rescript 12.0.0-beta.6 .

The following:

"12xxx3"->Int.fromString  

parses to Some(12). This is unexpected for me, because I would expect that it parses to None, since "12xxx3" is not a valid integer.

Is this intentional, or a bug? I would consider this a bug.

[zth]

Good question. I don't remember exactly how that behavior is defined. Just to clarify - is this still using Core, or is it using the stdlib directly?

[fhammerschmidt]

It does not matter which Stdlib. Belt, Core and v12 Stdlib all use parseInt under the hood which does not return NaN for "12xxx3" either.

> console.log(parseInt("12xxx3"));
12

From MDN:

If parseInt encounters a character in the input string that is not a valid numeral in the specified radix, it ignores it and all succeeding characters and returns the integer value parsed up to that point.

Not sure if it's worth it for us to check the string with a regex before parsing, or even decide to implement the whole parsing in a stricter way ourselves.

[reebalazs]

I’m using Core (implicitly imported), but yes: I checked, and the stdlib behaves the same way.

I suspected this behavior might actually be intentional, since it just delegates to JavaScript’s parseInt. I’d even be fine with that if the function were called parseInt. But it’s named fromString, and one wouldn’t expect this kind of behavior from a fromString. All the other fromString functions either return an option or throw an exception. (Fww, BigInt has both a fromString and a fromStringExn which btw should now be called fromStringOrThrow...)

In my wrapper, I currently convert the result back to a string with toString and compare it for equality, though using a smart regular expression or even a custom implementation might be slightly more efficient.

[glennsl]

Related: #86, #93 and rescript-lang/rescript#3732

[zth]

I like this proposed solution @glennsl #86 (comment)

If we want to do this then now is the time, as we're shipping the new stdlib in v12 stable.

[reebalazs]

I also like #86 (comment)

cc @glennsl