Skip to content
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

toInt/toInt64 is wrong #112

Open
liuaiyuan opened this issue Apr 21, 2021 · 4 comments
Open

toInt/toInt64 is wrong #112

liuaiyuan opened this issue Apr 21, 2021 · 4 comments

Comments

@liuaiyuan
Copy link

liuaiyuan commented Apr 21, 2021

var value = "00100"
fmt.Println(strconv.ParseInt(value, 10, 64))
fmt.Println(strconv.Atoi(value))
fmt.Println(cast.ToInt64E(value))
fmt.Println(cast.ToIntE(value))

output

100 <nil> ✅
100 <nil> ✅
64 <nil> ❌
64 <nil>
var value = "012345678"
fmt.Println(strconv.ParseInt(value, 10, 64))
fmt.Println(strconv.Atoi(value))
fmt.Println(cast.ToInt64E(value))
fmt.Println(cast.ToIntE(value))

output

12345678 <nil> ✅
12345678 <nil> ✅
0 unable to cast "012345678" of type string to int64 ❌
0 unable to cast "012345678" of type string to int ❌
@bep
Copy link
Collaborator

bep commented Apr 21, 2021

The "is wrong" statement is a little bombastic -- but the current behaviour is certainly a little undocumented. But it's certainly in line with the corresponding documentation in Go's stdlib.

https://play.golang.org/p/fMXB0rEgN9Z

@liuaiyuan
Copy link
Author

The "is wrong" statement is a little bombastic -- but the current behaviour is certainly a little undocumented. But it's certainly in line with the corresponding documentation in Go's stdlib.

https://play.golang.org/p/fMXB0rEgN9Z

Okay, thank you very much for your reply, but for special values, this will not apply and the developer needs to deal with it by himself

@bep
Copy link
Collaborator

bep commented Apr 22, 2021

@liuaiyuan this isn't my library and I did not implement these, and I would probably also prefer that they behaved the way you describe, but that would probably be too much of a breaking change.

/cc @spf13

@dwburke
Copy link

dwburke commented Apr 22, 2021

you could create a pr :)

the problem could be fixed with changing the v, err := strconv.ParseInt(s, 0, 0) 's to v, err := strconv.ParseInt(s, 10, 0) ...

I did this and added to the tests for your scenario, and it works... what makes me hesitate to submit a pr is I do not know what this breaks for people, even though this "fix" is how I would expect it to work there's probably a lot I'm not considering...

I modified all the "to" functions and tests, but this is part of the diff to illustrate my meaning

@ -345,6 +349,10 @@ func TestToInt64E(t *testing.T) {
                {true, 1, false},
                {false, 0, false},
                {"8", 8, false},
+               {"-8", -8, false},
+               {"0100", 100, false},
+               {"01234", 1234, false},
+               {"1234", 1234, false},
                {nil, 0, false},
                // errors
                {"test", 0, true},
diff --git a/caste.go b/caste.go
index 70c7291..6ccc2d0 100644
--- a/caste.go
+++ b/caste.go
@@ -211,7 +211,7 @@ func ToInt64E(i interface{}) (int64, error) {
        case float32:
                return int64(s), nil
        case string:
-               v, err := strconv.ParseInt(s, 0, 0)
+               v, err := strconv.ParseInt(s, 10, 0)
                if err == nil {
                        return v, nil
                }

the new tests fail until parseint is changed, then they all succeed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants