From 5ab1cb976731934074a14c68ea746c737044aac0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?RFish=20=E2=9A=93?= Date: Sat, 3 Jul 2021 16:53:52 +0200 Subject: [PATCH] [Belt] Stricter parse for Int and Float 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 #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 --- jscomp/others/belt_Float.ml | 2 +- jscomp/others/belt_Int.ml | 2 +- jscomp/test/bs_float_test.ml | 11 ++++++++++- jscomp/test/bs_int_test.ml | 11 ++++++++++- 4 files changed, 22 insertions(+), 4 deletions(-) diff --git a/jscomp/others/belt_Float.ml b/jscomp/others/belt_Float.ml index ed9c8fc7eb..c5da4af801 100644 --- a/jscomp/others/belt_Float.ml +++ b/jscomp/others/belt_Float.ml @@ -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 diff --git a/jscomp/others/belt_Int.ml b/jscomp/others/belt_Int.ml index c03ce007c1..f6bb92685e 100644 --- a/jscomp/others/belt_Int.ml +++ b/jscomp/others/belt_Int.ml @@ -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 diff --git a/jscomp/test/bs_float_test.ml b/jscomp/test/bs_float_test.ml index f6e4efb244..aa780ded57 100644 --- a/jscomp/test/bs_float_test.ml +++ b/jscomp/test/bs_float_test.ml @@ -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"; diff --git a/jscomp/test/bs_int_test.ml b/jscomp/test/bs_int_test.ml index 0e90c5e0c1..b1cf58f5f0 100644 --- a/jscomp/test/bs_int_test.ml +++ b/jscomp/test/bs_int_test.ml @@ -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";