fix(strutil): return zero digit instead of empty string in base conversion
BaseConvIntByTpl returned an empty string for the value zero because its loop condition (dec > 0) was never entered. This affected BaseConv, Base10Conv and BaseConvInt for bases above 62, and was inconsistent with BaseConvInt for smaller bases which correctly returned "0" via strconv. Emit the zero digit of the target base for a zero input and guard against an empty template.
This commit is contained in:
1 parent
17b49be136
commit
d6c91f3cdd
3 files changed
+99
No files matched your search
@@ -0,0 +1,74 @@
|
||||
# fix(strutil): BaseConv family returns "0" for zero instead of empty string
|
||||
|
||||
## Problem
|
||||
|
||||
Converting the value zero through the base-conversion helpers returns an empty
|
||||
string instead of `"0"`:
|
||||
|
||||
```go
|
||||
strutil.BaseConv("0", 10, 16) // "" , want "0"
|
||||
strutil.BaseConv("0", 16, 10) // "" , want "0"
|
||||
strutil.Base10Conv("0", 62) // "" , want "0"
|
||||
strutil.BaseConvInt(0, 64) // "" , want "0"
|
||||
```
|
||||
|
||||
This is also internally inconsistent: `BaseConvInt` returns the correct `"0"`
|
||||
for small bases because it delegates to `strconv.FormatUint`/`big.Int.Text`,
|
||||
but returns `""` for bases in `(62, 64]`, which go through the template path:
|
||||
|
||||
```go
|
||||
strutil.BaseConvInt(0, 16) // "0" (strconv path)
|
||||
strutil.BaseConvInt(0, 64) // "" (template path)
|
||||
```
|
||||
|
||||
## Root cause
|
||||
|
||||
`strutil/convbase.go`, `BaseConvIntByTpl`:
|
||||
|
||||
```go
|
||||
func BaseConvIntByTpl(dec uint64, toBase string) string {
|
||||
var res string
|
||||
toLen := uint64(len(toBase))
|
||||
for dec > 0 { // never runs when dec == 0
|
||||
res = string(toBase[dec%toLen]) + res
|
||||
dec /= toLen
|
||||
}
|
||||
return res // returns "" for dec == 0
|
||||
}
|
||||
```
|
||||
|
||||
The loop condition `dec > 0` is never entered when `dec == 0`, so the function
|
||||
returns the zero-value empty string. Every public entry point that reaches the
|
||||
template path (`BaseConvIntByTpl`, `BaseConvByTpl`, `BaseConv`, `Base10Conv`,
|
||||
`BaseConvInt` for base > 62) inherits the defect.
|
||||
|
||||
## Fix
|
||||
|
||||
Handle the zero case explicitly by emitting the zero digit of the target base
|
||||
(the first character of the template), and guard against an empty template to
|
||||
avoid an out-of-range index:
|
||||
|
||||
```go
|
||||
if toLen == 0 {
|
||||
return res
|
||||
}
|
||||
if dec == 0 {
|
||||
return string(toBase[0])
|
||||
}
|
||||
```
|
||||
|
||||
## Test
|
||||
|
||||
Added `TestBaseConv_zero` in `strutil/convbase_test.go` covering `BaseConv`,
|
||||
`Base10Conv`, `BaseConvInt` (small and large bases) and `BaseConvIntByTpl`.
|
||||
|
||||
- Gate: `go test ./strutil/ -run 'TestBaseConv|TestBase10Conv'` -> ok (PASS)
|
||||
- `go vet ./strutil/` -> exit 0
|
||||
- `go build ./...` -> exit 0
|
||||
|
||||
Counterfactual: reverting the change in `BaseConvIntByTpl` makes the new test
|
||||
fail with `actual: string("")` vs `expect: string("0")`.
|
||||
|
||||
Note: `TestDateSNv3/dateLen4_base62` in `strutil/gensn_test.go` is flaky and
|
||||
already fails on the unmodified base commit (it asserts uniqueness of randomly
|
||||
generated serial numbers over 60 iterations); it is unrelated to this change.
|
||||
@@ -100,6 +100,16 @@ func BaseConvIntByTpl(dec uint64, toBase string) string {
|
||||
// convert to new base
|
||||
var res string
|
||||
toLen := uint64(len(toBase))
|
||||
if toLen == 0 {
|
||||
return res
|
||||
}
|
||||
|
||||
// zero maps to the zero digit of the target base, otherwise the loop below
|
||||
// would produce an empty string for a valid input.
|
||||
if dec == 0 {
|
||||
return string(toBase[0])
|
||||
}
|
||||
|
||||
for dec > 0 {
|
||||
res = string(toBase[dec%toLen]) + res
|
||||
dec /= toLen
|
||||
|
||||
@@ -7,6 +7,21 @@ import (
|
||||
"github.com/gookit/goutil/x/assert"
|
||||
)
|
||||
|
||||
func TestBaseConv_zero(t *testing.T) {
|
||||
// Zero must convert to the zero digit of the target base, not an empty
|
||||
// string. This also keeps BaseConvInt consistent across all bases: the
|
||||
// strconv-backed path already returns "0" for small bases.
|
||||
assert.Eq(t, "0", strutil.BaseConv("0", 10, 16))
|
||||
assert.Eq(t, "0", strutil.BaseConv("0", 16, 10))
|
||||
assert.Eq(t, "0", strutil.Base10Conv("0", 62))
|
||||
|
||||
assert.Eq(t, "0", strutil.BaseConvInt(0, 16))
|
||||
assert.Eq(t, "0", strutil.BaseConvInt(0, 62))
|
||||
assert.Eq(t, "0", strutil.BaseConvInt(0, 64))
|
||||
|
||||
assert.Eq(t, "0", strutil.BaseConvIntByTpl(0, strutil.Base64Chars))
|
||||
}
|
||||
|
||||
func TestBaseConv(t *testing.T) {
|
||||
tests := []struct {
|
||||
give string
|
||||
|
||||
Reference in new issue
Block a user