Skip to content

Commit

Permalink
[Belt] Stricter parse for Int and Float
Browse files Browse the repository at this point in the history
The actual implementation of Belt.Int.fromString relies on the
Javascript `parseInt` function [0].  That function parses strings in a
permissive manner.  For instance, if the function encounters an
alphabetical character, it returns the value up to that character:

    parseInt("123a456") // output: 123

The same applies for Belt.Float.fromString that relies on
`parseFloat` [1] and have a similar interpretation.

This commit proposes a stricter implementation of Int and Float parsing
using the Javascript `Number` constructor [2].  The Belt.Int.fromString
uses the more specific `Math.trunc` function [3], that parses string
similarly to `Number` and then returns the Int part of that number.

Relying only on the `Number` constructor keeps the implementation
simple.  However, there exists one last quirk while using it with an
empty string.  It returns the `0` value:

    Number("") // ouptput: 0

Consequently, Belt.Int.fromString("") will return `0`.  Tests for
Belt.(Int/Float).fromString have been updated in order to reflect that.

Fix rescript-lang#3732

[0] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt
[1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseFloat
[2] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/Number
[3] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/trunc

Signed-Off-By: Ronan-A. Cherrueau <[email protected]>
  • Loading branch information
rcherrueau committed Jul 3, 2021
1 parent bf887f1 commit 5ab1cb9
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 4 deletions.
2 changes: 1 addition & 1 deletion jscomp/others/belt_Float.ml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ external toInt: float -> int = "%intoffloat"

external fromInt: int -> float = "%identity"

external fromString: string -> float = "parseFloat" [@@bs.val]
external fromString: string -> float = "Number" [@@bs.val]

let fromString i =
match (fromString i) with
Expand Down
2 changes: 1 addition & 1 deletion jscomp/others/belt_Int.ml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ external toFloat: int -> float = "%identity"

external fromFloat: float -> int = "%intoffloat"

external fromString: string -> (_ [@bs.as 10]) -> int = "parseInt" [@@bs.val]
external fromString: string -> (_ [@bs.as 10]) -> int = "Math.trunc" [@@bs.val]

let fromString i =
match fromString i with
Expand Down
11 changes: 10 additions & 1 deletion jscomp/test/bs_float_test.ml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,16 @@ let () =
eq __LOC__ (F.fromString "-1.0") (Some (-1.0));
eq __LOC__ (F.fromString "-1.5") (Some (-1.5));
eq __LOC__ (F.fromString "-1.7") (Some (-1.7));
eq __LOC__ (F.fromString "not a float") None
eq __LOC__ (F.fromString "17e-1") (Some (1.7));
eq __LOC__ (F.fromString "-17e-1") (Some (-1.7));
eq __LOC__ (F.fromString " -17e-1 ") (Some (-1.7));
eq __LOC__ (F.fromString "0x11") (Some (17.));
eq __LOC__ (F.fromString "0b11") (Some (3.));
eq __LOC__ (F.fromString "0o11") (Some (9.));
eq __LOC__ (F.fromString "") (Some (0.));
eq __LOC__ (F.fromString "not a float") None;
eq __LOC__ (F.fromString "100.0abcdef") None;
eq __LOC__ (F.fromString "123_456.7") None

let () =
eq __LOC__ (F.toString 1.0) "1";
Expand Down
11 changes: 10 additions & 1 deletion jscomp/test/bs_int_test.ml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,16 @@ let () =
eq __LOC__ (I.fromString "-1.0") (Some (-1));
eq __LOC__ (I.fromString "-1.5") (Some (-1));
eq __LOC__ (I.fromString "-1.7") (Some (-1));
eq __LOC__ (I.fromString "not an int") None
eq __LOC__ (I.fromString "17e-1") (Some (1));
eq __LOC__ (I.fromString "-17e-1") (Some (-1));
eq __LOC__ (I.fromString " -17e-1 ") (Some (-1));
eq __LOC__ (I.fromString "0x11") (Some (17));
eq __LOC__ (I.fromString "0b11") (Some (3));
eq __LOC__ (I.fromString "0o11") (Some (9));
eq __LOC__ (I.fromString "") (Some (0));
eq __LOC__ (I.fromString "not an int") None;
eq __LOC__ (I.fromString "100abcdef") None;
eq __LOC__ (I.fromString "123_456") None

let () =
eq __LOC__ (I.toString 1) "1";
Expand Down

0 comments on commit 5ab1cb9

Please sign in to comment.