fix: ISBN-10 Generate outer mod-11 and restrict X to check-digit position

isbn.go

@@ -12,10 +12,14 @@ func (i10 isbn10) Verify(code string) bool {
 	}
 
 	sum, multiply := 0, 10
-	for _, n := range code {
+	for i, n := range code {
 		var digit int
 		switch {
+		// 'X' (value 10) is valid only as the check digit (final position).
 		case n == 'X':
+			if i != len(code)-1 {
+				return false
+			}
 			digit = 10
 		case isNotNumber(n):
 			return false
@@ -48,7 +52,9 @@ func (i10 *isbn10) Generate(seed string) (int, error) {
 		multiply--
 	}
 
-	return 11 - sum%11, nil
+	// ISBN-10 check digit (ISO 2108): (11 - (sum mod 11)) mod 11, range 0-10.
+	// The outer mod 11 maps the sum%11==0 case to 0 instead of an out-of-range 11.
+	return (11 - sum%11) % 11, nil
 }
 
 // Verify implements checkdigit.Verifier interface.

isbn_edge_test.go

@@ -0,0 +1,191 @@
+package checkdigit_test
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/osamingo/checkdigit"
+)
+
+// TestIsbn10_Generate_Mod11Zero covers the sum%11==0 boundary where the
+// check digit must be 0 (not the out-of-range 11 produced by the buggy
+// `11 - sum%11` expression that missed the outer `% 11`).
+func TestIsbn10_Generate_Mod11Zero(t *testing.T) {
+	t.Parallel()
+
+	cases := map[string]struct {
+		in  string
+		out int
+	}{
+		"All zeros": {
+			in:  "000000000",
+			out: 0,
+		},
+		"Weighted sum divisible by 11": {
+			in:  "055555555",
+			out: 0,
+		},
+	}
+
+	for name, c := range cases {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			r, err := checkdigit.NewISBN10().Generate(c.in)
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if c.out != r {
+				t.Errorf("not equal, expected = %d, given = %d", c.out, r)
+			}
+			if r < 0 || r > 10 {
+				t.Errorf("check digit out of range [0,10]: %d", r)
+			}
+		})
+	}
+}
+
+// TestIsbn10_Generate_NonZeroRegression guards the non-zero check-digit
+// cases (sum%11 != 0) that were already correct and must remain unchanged.
+func TestIsbn10_Generate_NonZeroRegression(t *testing.T) {
+	t.Parallel()
+
+	cases := map[string]struct {
+		in  string
+		out int
+	}{
+		"Check digit 9": {
+			in:  "000000001",
+			out: 9,
+		},
+		"Check digit 10 (X)": {
+			in:  "000000006",
+			out: 10,
+		},
+		"Arbitrary seed -> 10 (X)": {
+			in:  "123456789",
+			out: 10,
+		},
+	}
+
+	for name, c := range cases {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			r, err := checkdigit.NewISBN10().Generate(c.in)
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if c.out != r {
+				t.Errorf("not equal, expected = %d, given = %d", c.out, r)
+			}
+		})
+	}
+}
+
+// TestIsbn10_Verify_XWrongPosition covers the false-accept defect where
+// 'X' (value 10) was accepted in any position. 'X' is valid only as the
+// check digit (the final character) per ISO 2108.
+func TestIsbn10_Verify_XWrongPosition(t *testing.T) {
+	t.Parallel()
+
+	cases := map[string]struct {
+		in  string
+		out bool
+	}{
+		"X at first position": {
+			in:  "X026515627",
+			out: false,
+		},
+		"X at second position": {
+			in:  "0X00000009",
+			out: false,
+		},
+	}
+
+	for name, c := range cases {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			ret := checkdigit.NewISBN10().Verify(c.in)
+			if c.out != ret {
+				t.Errorf("not equal, expected = %v, given = %v", c.out, ret)
+			}
+		})
+	}
+}
+
+// TestIsbn10_Verify_ValidXLastPosition confirms that a valid ISBN-10 with
+// 'X' in the check-digit position is still accepted, plus basic
+// valid/invalid controls.
+func TestIsbn10_Verify_ValidXLastPosition(t *testing.T) {
+	t.Parallel()
+
+	cases := map[string]struct {
+		in  string
+		out bool
+	}{
+		"Valid X in last position": {
+			in:  "000000006X",
+			out: true,
+		},
+		"Valid all zeros": {
+			in:  "0000000000",
+			out: true,
+		},
+		"Invalid check digit": {
+			in:  "0000000001",
+			out: false,
+		},
+	}
+
+	for name, c := range cases {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			ret := checkdigit.NewISBN10().Verify(c.in)
+			if c.out != ret {
+				t.Errorf("not equal, expected = %v, given = %v", c.out, ret)
+			}
+		})
+	}
+}
+
+// TestIsbn10_RoundTrip checks the Generate -> Verify round-trip property
+// over a range of seeds, including the sum%11==0 boundary cases.
+func TestIsbn10_RoundTrip(t *testing.T) {
+	t.Parallel()
+
+	prov := checkdigit.NewISBN10()
+
+	seeds := []string{
+		"000000000", // sum%11==0 -> check digit 0
+		"055555555", // sum%11==0 -> check digit 0
+		"000000001", // -> 9
+		"000000006", // -> 10 (X)
+		"123456789", // -> 10 (X)
+		"002651562", // -> 8
+		"007231592", // -> 10 (X)
+		"155860832", // -> 10 (X)
+	}
+
+	for _, seed := range seeds {
+		t.Run(seed, func(t *testing.T) {
+			t.Parallel()
+
+			cd, err := prov.Generate(seed)
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			var code string
+			if cd == 10 {
+				code = seed + "X"
+			} else {
+				code = fmt.Sprintf("%s%d", seed, cd)
+			}
+			if !prov.Verify(code) {
+				t.Errorf("round-trip failed: Generate(%q)=%d -> Verify(%q)=false", seed, cd, code)
+			}
+		})
+	}
+}