fix: Parse enforces the decimal grammar itself, so "--5" is not +5

Parse stripped one leading sign and let big.Int.SetString read what was
left. SetString accepts a sign too, so a second one was consumed there:
"--5" parsed as POSITIVE five. On the values this package exists to hold
that is a credit read as a charge, and it is silent — the caller gets a
well-formed Decimal with no error to notice.

The same gap had a second side. A bare "-", "+" or "." reached the
intPart=="" → "0" fallback with no digits at all and parsed as a
confident zero, indistinguishable from a real "0".

Both come from delegating the grammar check to big.Int, which answers a
different question than "is this a decimal literal". Checking the digits
here answers that one, and it is the only check the grammar needs.

Co-authored-by: Hanzo Dev <[email protected]>
This commit is contained in:
2026-08-01 22:05:17 -07:00
co-authored by Hanzo Dev
parent 44ccd24fc0
commit e7ede18d50
2 changed files with 142 additions and 15 deletions
+32 -15
View File
@@ -74,39 +74,56 @@ func NewFromBig(coef *big.Int, scale int32) Decimal {
// Parse reads a decimal string ("123", "-0.00132", "+6.60") EXACTLY — no float. The
// scale is the number of fractional digits written (so "6.60" has scale 2). An empty
// string is 0.
//
// The grammar is exactly [+|-] digits [. digits] with at least one digit, and it is
// enforced by checking the digits rather than by asking big.Int to. Delegating the
// check let malformed input through in the worst possible way: this function strips
// ONE leading sign, and big.Int.SetString then accepted a SECOND one, so "--5"
// parsed as +5 — a sign inversion on a value that is usually money, where a credit
// silently becomes a charge. A bare "-", "+" or "." fell through the other side of
// the same gap, taking the intPart=="" → "0" fallback and parsing as a confident
// zero. Both now return an error.
func Parse(s string) (Decimal, error) {
s = strings.TrimSpace(s)
if s == "" {
return Decimal{}, nil
}
neg := false
switch s[0] {
body, neg := s, false
switch body[0] {
case '+':
s = s[1:]
body = body[1:]
case '-':
neg, s = true, s[1:]
neg, body = true, body[1:]
}
intPart, fracPart := s, ""
if i := strings.IndexByte(s, '.'); i >= 0 {
intPart, fracPart = s[:i], s[i+1:]
}
if intPart == "" {
intPart = "0"
intPart, fracPart := body, ""
if i := strings.IndexByte(body, '.'); i >= 0 {
intPart, fracPart = body[:i], body[i+1:]
}
digits := intPart + fracPart
if digits == "" {
return Decimal{}, fmt.Errorf("decimal: invalid %q", s)
}
c, ok := new(big.Int).SetString(digits, 10)
if !ok {
if !allDigits(digits) {
return Decimal{}, fmt.Errorf("decimal: invalid %q", s)
}
c, _ := new(big.Int).SetString(digits, 10)
if neg {
c.Neg(c)
}
return Decimal{coef: c, scale: int32(len(fracPart))}, nil
}
// allDigits reports whether s is one or more ASCII digits and nothing else — no
// sign, no separator, no exponent, no unicode digit that strconv would widen.
func allDigits(s string) bool {
if s == "" {
return false
}
for i := 0; i < len(s); i++ {
if s[i] < '0' || s[i] > '9' {
return false
}
}
return true
}
// MustParse is Parse that panics on error — for constants/tests.
func MustParse(s string) Decimal {
d, err := Parse(s)
+110
View File
@@ -0,0 +1,110 @@
// Copyright (c) 2026, Hanzo AI, Inc. BSD-3-Clause.
package decimal
import (
"strconv"
"testing"
)
// TestParseRejectsMalformed pins the grammar. Every case here USED TO PARSE, and the
// two families it covers are the two ways an exact parser can be worse than a float
// one: a wrong SIGN and a confident ZERO.
func TestParseRejectsMalformed(t *testing.T) {
for _, s := range []string{
// Double sign. "--5" parsed as +5: this function stripped one '-' and
// big.Int.SetString consumed the other, so a credit read as a charge.
"--5", "+-5", "-+5", "++5", "--5.00", "--0.01",
// A sign or a point with no digits at all. These took the intPart==""
// fallback and returned a zero that nothing distinguished from "0".
"-", "+", ".", "-.", "+.", "-+", ".-",
// Sign in the wrong place.
"5-", "5+", "1.-2", "1.+2",
// Not a decimal literal.
"abc", "1.2.3", "1,234", "1_000", "0x10", "1e3", "1.2e3",
"NaN", "Inf", "-Inf", "٥", "5",
} {
t.Run(strconv.Quote(s), func(t *testing.T) {
d, err := Parse(s)
if err == nil {
t.Fatalf("Parse(%q) = %s with no error — malformed input became a value", s, d.String())
}
if !d.IsZero() {
t.Fatalf("Parse(%q) returned %s alongside an error", s, d.String())
}
})
}
}
// TestParseAcceptsWellFormed is the converse guard: the grammar check must not have
// narrowed what Parse legitimately reads. Every case is a literal a real caller
// writes, including the sparse forms around the decimal point.
func TestParseAcceptsWellFormed(t *testing.T) {
cases := []struct{ in, want string }{
{"0", "0"},
{"5", "5"},
{"-5", "-5"},
{"+5", "5"},
{"123", "123"},
{"19.99", "19.99"},
{"-19.99", "-19.99"},
{"+6.60", "6.6"}, // String trims the trailing zero; the SCALE keeps it
{"-0.00132", "-0.00132"},
{".5", "0.5"}, // no integer part
{"-.5", "-0.5"},
{"+.5", "0.5"},
{"1.", "1"}, // no fractional part
{"-1.", "-1"},
{"0.00", "0"},
{"-0.00", "0"}, // negative zero is zero
{" 42.42 ", "42.42"}, // surrounding space is trimmed
{"000123", "123"},
{"1234567890123456789012345678901234567890", "1234567890123456789012345678901234567890"},
}
for _, tc := range cases {
t.Run(strconv.Quote(tc.in), func(t *testing.T) {
d, err := Parse(tc.in)
if err != nil {
t.Fatalf("Parse(%q) errored: %v", tc.in, err)
}
if got := d.String(); got != tc.want {
t.Fatalf("Parse(%q) = %s, want %s", tc.in, got, tc.want)
}
})
}
}
// TestParseKeepsWrittenScale pins what the doc actually promises about trailing
// zeros: the SCALE is the number of fractional digits written, even where String
// trims them for display. Rescale to minor units reads the scale, not the rendering.
func TestParseKeepsWrittenScale(t *testing.T) {
for _, tc := range []struct {
in string
want int32
}{
{"6.60", 2}, {"6.6", 1}, {"6", 0}, {"6.", 0},
{"0.00", 2}, {"-0.00132", 5}, {".5", 1},
} {
d, err := Parse(tc.in)
if err != nil {
t.Fatalf("Parse(%q) errored: %v", tc.in, err)
}
if got := d.Scale(); got != tc.want {
t.Fatalf("Parse(%q).Scale() = %d, want %d", tc.in, got, tc.want)
}
}
}
// TestParseSignIsNeverInverted states the property behind the first family directly:
// no input may produce a value whose sign disagrees with the sign written on it.
func TestParseSignIsNeverInverted(t *testing.T) {
for _, s := range []string{"-5", "-0.01", "-1234.56", "--5", "-+5", "+-5"} {
d, err := Parse(s)
if err != nil {
continue // refusing is always an acceptable answer
}
if d.Sign() > 0 {
t.Fatalf("Parse(%q) = %s — a negative literal parsed POSITIVE", s, d.String())
}
}
}