rescript-core icon indicating copy to clipboard operation
rescript-core copied to clipboard

Int.fromString unexpected behavior

Open reebalazs opened this issue 2 months ago • 6 comments

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.

reebalazs avatar Oct 20 '25 09:10 reebalazs

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?

zth avatar Oct 20 '25 09:10 zth

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.

fhammerschmidt avatar Oct 20 '25 11:10 fhammerschmidt

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.

reebalazs avatar Oct 20 '25 14:10 reebalazs

Related: #86, #93 and https://github.com/rescript-lang/rescript/issues/3732

glennsl avatar Oct 21 '25 12:10 glennsl

I like this proposed solution @glennsl https://github.com/rescript-lang/rescript-core/issues/86#issuecomment-1454708245

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

zth avatar Oct 21 '25 12:10 zth

I also like https://github.com/rescript-lang/rescript-core/issues/86#issuecomment-1454708245

cc @glennsl

reebalazs avatar Oct 22 '25 11:10 reebalazs