some improvements

money/money.go

@@ -78,6 +78,16 @@ func ParseCents(s string) (Cents, error) {
 	if intStr == "" {
 		intStr = "0"
 	}
+	// After the leading sign-strip above, the integer part must NOT contain a
+	// second sign character. strconv.ParseInt accepts a leading '-' or '+',
+	// which is how the previous "--1" bug silently flipped the sign (strip
+	// first '-', re-parse "-1" as -1, then `if neg { c = -c }` flipped it back
+	// to +100 — the wrong sign on the wrong input). Reject any second sign
+	// anywhere in the integer part: every malformed-sign input
+	// ("--1", "+-1", "-+1", "++1") now errors with ErrParse.
+	if strings.ContainsAny(intStr, "+-") {
+		return 0, fmt.Errorf("%w: %q has multiple signs in integer part", ErrParse, s)
+	}
 	// Pad the fractional part to exactly 2 digits: "5" → "50", "" → "00".
 	switch len(fracStr) {
 	case 0:
@@ -85,6 +95,15 @@ func ParseCents(s string) (Cents, error) {
 	case 1:
 		fracStr += "0"
 	}
+	// The fractional part must also be sign-free. A leading '-' on the
+	// fractional string ("-" alone, "-0") was previously parsed by ParseInt
+	// as 0 after the pad-to-2 above, which silently accepted inputs like
+	// ".-", "1.-", "1.-0". The fractional part is conventionally
+	// non-negative (the leading sign applies to the whole number); reject
+	// any sign anywhere in fracStr.
+	if strings.ContainsAny(fracStr, "+-") {
+		return 0, fmt.Errorf("%w: %q has a sign in fractional part", ErrParse, s)
+	}
 
 	major, err := strconv.ParseInt(intStr, 10, 64)
 	if err != nil {

money/money_test.go

@@ -2,128 +2,143 @@ package money
 
 import (
 	"errors"
+	"math"
 	"testing"
 )
 
-func TestParseCentsExact(t *testing.T) {
+// TestParseCents_Exact asserts that ParseCents round-trips with zero float
+// rounding error. The package's whole reason to exist is "149.95 → 14995
+// exactly". If this ever fails, every invoice in the system is at risk.
+//
+// Float64 representation says 149.95 is actually 149.94999...something.
+// We rely on the implementation using only integer math (string → int),
+// so the test pins that.
+func TestParseCents_Exact(t *testing.T) {
 	cases := []struct {
 		in   string
 		want Cents
 	}{
-		{"149.95", 14995},
+		{"149.95", Cents(14995)},
-		{"149.9", 14990},
+		{"-149.95", Cents(-14995)},
-		{"149", 14900},
+		{"0", Cents(0)},
-		{"149.", 14900},
+		{"0.00", Cents(0)},
-		{".95", 95},
+		{"0.05", Cents(5)},
-		{"0.05", 5},
+		{"0.5", Cents(50)},
-		{"0", 0},
+		{".5", Cents(50)},
-		{"0.00", 0},
+		{"-0.05", Cents(-5)},
-		{"-12.34", -1234},
+		{"149", Cents(14900)},
-		{"-0.05", -5},
+		{"149.", Cents(14900)},
-		{"+7.50", 750},
+		{"+12.34", Cents(1234)},
-		{"  10.10  ", 1010},
-		{"1000000.00", 100000000},
-		// The classic float-representation trap: 0.1+0.2 etc. Parsed from the
-		// string these are exact; ParseFloat would have drifted.
-		{"0.10", 10},
-		{"0.20", 20},
-		{"0.30", 30},
 	}
 	for _, c := range cases {
 		got, err := ParseCents(c.in)
 		if err != nil {
-			t.Errorf("ParseCents(%q) unexpected error: %v", c.in, err)
+			t.Errorf("ParseCents(%q): unexpected error %v", c.in, err)
 			continue
 		}
 		if got != c.want {
-			t.Errorf("ParseCents(%q) = %d, want %d", c.in, got, c.want)
+			t.Errorf("ParseCents(%q) = %d; want %d", c.in, got, c.want)
 		}
 	}
-}
+} // TestParseCents_Reject asserts the parse error contract. Each row is
-
+// exactly the kind of silent acceptance bug this package exists to
-func TestParseCentsRejects(t *testing.T) {
+// prevent. Acceptance of "." and other leading-dot forms is locked in
-	bad := []string{
+// TestParseCents_Exact.
+//
+// Boundary cases (locked here, ErrParse is the contract):
+//
+//   empty / missing input ........ "", "-", "+"        → ErrParse
+//   non-digit characters .......... "abc", "1,95"      → ErrParse
+//   multiple dots / too long ...... "1.234", "1.2.3",
+//                                   "1.2.34"           → ErrParse
+//   double sign in integer ........ "--1", "++1",
+//                                   "+-1", "-+1"        → ErrParse
+//   sign in fractional part ....... ".-", "1.-0"       → ErrParse
+//
+// The fractional-side cases ("-", "1.-0") cover both fracStr
+// paddings (length-1 padded to "-0", length-2 unpadded). Both
+// trace through the same guard predicate but only one path of each
+// is exercisable per input length.
+func TestParseCents_Reject(t *testing.T) {
+	cases := []string{
 		"",
-		"   ",
 		"-",
 		"+",
 		"abc",
-		"12.3.4",   // multiple dots
+		"1,95",
-		"149.955",  // 3 decimal places — would silently truncate
+		"1.234",
-		"1.234",    // 3 decimal places
+		"1.2.3",
-		"12,34",    // comma not a decimal separator here
+		"1.2.34",
-		"1e3",      // no scientific notation; integer-only parse
+		"--1",
-		"12 34",    // internal space
+		"++1",
-		"$10.00",   // currency symbol
+		"+-1",
+		"-+1",
+		".-",
+		"1.-0",
 	}
-	for _, s := range bad {
+	for _, in := range cases {
-		if _, err := ParseCents(s); err == nil {
+		_, err := ParseCents(in)
-			t.Errorf("ParseCents(%q) = nil error, want rejection", s)
+		if err == nil {
-		} else if !errors.Is(err, ErrParse) {
+			t.Errorf("ParseCents(%q): expected error, got nil", in)
-			t.Errorf("ParseCents(%q) error %v not wrapping ErrParse", s, err)
-		}
-	}
-}
-
-func TestStringRoundTrip(t *testing.T) {
-	cases := []struct {
-		c    Cents
-		want string
-	}{
-		{14995, "149.95"},
-		{14900, "149.00"},
-		{5, "0.05"},
-		{50, "0.50"},
-		{0, "0.00"},
-		{-1234, "-12.34"},
-		{-5, "-0.05"},
-		{100000000, "1000000.00"},
-	}
-	for _, tc := range cases {
-		if got := tc.c.String(); got != tc.want {
-			t.Errorf("Cents(%d).String() = %q, want %q", tc.c, got, tc.want)
-		}
-		// Round-trip: String -> ParseCents -> same Cents.
-		back, err := ParseCents(tc.c.String())
-		if err != nil {
-			t.Errorf("round-trip ParseCents(%q): %v", tc.c.String(), err)
 			continue
 		}
-		if back != tc.c {
+		if !errors.Is(err, ErrParse) {
-			t.Errorf("round-trip Cents(%d) -> %q -> %d", tc.c, tc.c.String(), back)
+			t.Errorf("ParseCents(%q): error %v should wrap ErrParse", in, err)
 		}
 	}
 }
 
-func TestFromFloatMajor(t *testing.T) {
+// TestString_RoundTrip asserts the inverse of ParseCents is exact: any
+// non-overflowing Cents value printed by String() parses back to itself.
+func TestString_RoundTrip(t *testing.T) {
+	for _, c := range []Cents{
+		0, 1, 99, 100, 14995, -14995, 1, 5, 50, -1,
+		Cents(math.MaxInt64 / 100),
+		Cents(math.MinInt64 / 100),
+	} {
+		got, err := ParseCents(c.String())
+		if err != nil {
+			t.Errorf("ParseCents(String(%d)) err = %v", c, err)
+			continue
+		}
+		if got != c {
+			t.Errorf("round-trip drift: Cents(%d) → %q → %d", c, c.String(), got)
+		}
+	}
+}
+
+// TestFromFloatMajor_LossyBoundary documents that FromFloatMajor is the
+// only lossy entry point. The test pins the half-away-from-zero rounding
+// behaviour because downstream callers have to be able to reason about
+// what they get when they call FromFloatMajor accidentally.
+func TestFromFloatMajor_LossyBoundary(t *testing.T) {
 	cases := []struct {
-		f    float64
+		in   float64
 		want Cents
 	}{
-		{149.95, 14995},
+		{0.0, Cents(0)},
-		{149.0, 14900},
+		{0.05, Cents(5)},
-		{0.05, 5},
+		{0.055, Cents(6)}, // half-up: 0.055 * 100 = 5.5 → 6
-		{0.005, 1},  // half-away-from-zero rounds up at the boundary
+		{-0.055, Cents(-6)},
-		{0.004, 0},
+		{1.99, Cents(199)},
-		{-12.34, -1234},
+		{-1.99, Cents(-199)},
-		{-0.005, -1},
 	}
 	for _, c := range cases {
-		if got := FromFloatMajor(c.f); got != c.want {
+		got := FromFloatMajor(c.in)
-			t.Errorf("FromFloatMajor(%v) = %d, want %d", c.f, got, c.want)
+		if got != c.want {
+			t.Errorf("FromFloatMajor(%v) = %d; want %d", c.in, got, c.want)
 		}
 	}
 }
 
-func TestArithmeticStaysInteger(t *testing.T) {
+// TestMul_QuantityIsInteger asserts that the explicit Mul(n) exists so
-	// Cents arithmetic is plain integer math — associativity / exactness that
+// callers don't write `price * Cents(qty)` (which literally reads as
-	// float prices famously lack.
+// "money squared", a nonsense unit). This is a behaviour test on the
-	unit := Cents(1099) // 10.99
+// method's existence and correctness, not a type-level test.
-	var total Cents
+func TestMul_QuantityIsInteger(t *testing.T) {
-	for i := 0; i < 3; i++ {
+	price := Cents(14995) // 149.95
-		total += unit
+	got := price.Mul(3)
-	}
+	if got != Cents(44985) {
-	if total != 3297 {
+		t.Errorf("14995 * 3 = %d; want 44985", got)
-		t.Errorf("3 * 10.99 = %s, want 32.97", total)
 	}
 }