diff --git a/PR.md b/PR.md new file mode 100644 index 0000000..4c26d1c --- /dev/null +++ b/PR.md @@ -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. diff --git a/strutil/convbase.go b/strutil/convbase.go index d15fc53..6ae7ac4 100644 --- a/strutil/convbase.go +++ b/strutil/convbase.go @@ -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 diff --git a/strutil/convbase_test.go b/strutil/convbase_test.go index 86d9052..a03555f 100644 --- a/strutil/convbase_test.go +++ b/strutil/convbase_test.go @@ -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