Reject weight 0, a never-numeric calc operand and a repeat product past the cap; the CLI classifies ErrNoData

README.md

@@ -167,8 +167,8 @@ An item of a choice may carry a `weight` (default `1`) to skew its odds:
 
 Renders `070-412 38 91` ten times as often as `08-…`. A string item is weighted by
 writing it as `{ "format": "AB", "weight": 3 }`. Rejected at load: a weight that
-is negative, non-numeric, `1` (the default) or outside a choice, a set summing to
-zero, and a repeated item — `["a", "a", "b"]` is `[{ "format": "a", "weight": 2 }, "b"]`,
+is negative, non-numeric, `0` (never drawn — remove the item), `1` (the default)
+or outside a choice, and a repeated item — `["a", "a", "b"]` is `[{ "format": "a", "weight": 2 }, "b"]`,
 and the error says so.
 
 ### Repeat
@@ -180,7 +180,9 @@ many times — each an independent draw — joined by `separator` (default `""`)
 { "format": "{word}", "repeat": 3, "separator": " ", "word": ["foo", "bar", "baz"] }
 ```
 
-Renders e.g. `bar foo baz`. A `separator` without a `repeat` is rejected.
+Renders e.g. `bar foo baz`. A `separator` without a `repeat` is rejected, and so
+is a `repeat` that multiplies to more than 1 048 576 renders along any path of
+nested repeats.
 
 ### Options and fields
 
@@ -240,8 +242,9 @@ hyphenated field can't be an operand.
 { "format": "{net} x {qty} = {calc(net * qty, 2)}", "net": ["19.99", "5.00"], "qty": ["3", "7"] }
 ```
 
-Renders e.g. `19.99 x 3 = 59.97`. A non-numeric operand yields `NaN` and a
-division by zero `Inf`; both print rather than fail.
+Renders e.g. `19.99 x 3 = 59.97`. An operand that can never be a number (`"abc"`,
+or a choice of such) is rejected at load; one that sometimes is not yields `NaN`,
+and a division by zero `Inf` — both print rather than fail.
 
 ### Transforms
 

builtins.go

@@ -9,9 +9,10 @@ import (
 	"unicode"
 )
 
-// maxLen caps sample output lengths (hex, nanoid, base64) and maxDecimals caps
-// float/calc decimal places, so a fat-fingered or overflowing argument fails at
-// New instead of trying to allocate gigabytes — or panicking — at render.
+// maxLen caps sample output lengths (hex, nanoid, base64) and the renders a repeat
+// multiplies to along any path; maxDecimals caps float/calc decimal places. So a
+// fat-fingered or overflowing argument fails at New instead of trying to allocate
+// gigabytes — or panicking — at render.
 const (
 	maxLen      = 1 << 20 // 1,048,576 chars/bytes
 	maxDecimals = 1024

calc.go

@@ -70,9 +70,13 @@ func checkCalc(fields map[string]node, args []string) error {
 		return fmt.Errorf("calc(%q): %w", args[0], err)
 	}
 	for _, name := range calcVars(expr) {
-		if _, ok := fields[name]; !ok {
+		operand, ok := fields[name]
+		if !ok {
 			return fmt.Errorf("calc(%q): no field %q", args[0], name)
 		}
+		if text, never := neverNumeric(operand); never {
+			return fmt.Errorf("calc(%q): operand %q is never a number: it renders %q", args[0], name, text)
+		}
 	}
 	if len(args) == 2 {
 		if dp, err := strconv.Atoi(args[1]); err != nil || dp < 0 || dp > maxDecimals {
@@ -82,6 +86,30 @@ func checkCalc(fields map[string]node, args []string) error {
 	return nil
 }
 
+// neverNumeric reports a node no render of which is a number: fixed text that does
+// not parse, or a choice of only such items. text is one such render.
+func neverNumeric(n node) (text string, never bool) {
+	switch n := n.(type) {
+	case *template:
+		if !n.fixed || n.repeat > 1 {
+			return "", false
+		}
+		if _, err := strconv.ParseFloat(strings.TrimSpace(n.lit), 64); err != nil {
+			return n.lit, true
+		}
+	case *choice:
+		for _, it := range n.items {
+			t, itemNever := neverNumeric(it)
+			if !itemNever {
+				return "", false
+			}
+			text = t
+		}
+		return text, true
+	}
+	return "", false
+}
+
 // calcPrep parses the expression and decimals once, at compile time, and places each
 // operand name at the position expand will read it into. checkCalc proved both args
 // valid, so no step here can fail; dp -1 prints the minimal form.

calc_test.go

@@ -72,12 +72,16 @@ func TestCalcFields(t *testing.T) {
 	}
 }
 
-// TestCalcNonNumericIsNaN pins the never-fail rule: a field that doesn't render
-// to a number becomes NaN, which propagates and prints visibly.
+// TestCalcNonNumericIsNaN pins the never-fail rule: a field that sometimes does
+// not render to a number becomes NaN then, which propagates and prints visibly.
 func TestCalcNonNumericIsNaN(t *testing.T) {
-	if got := mustRender(t, engine(1), `{"format":"{calc(x * 2)}","x":"abc"}`); got != "NaN" {
-		t.Fatalf("calc over non-numeric field = %q, want NaN", got)
+	f := engine(1)
+	for i := 0; i < 50; i++ {
+		if got := mustRender(t, f, `{"format":"{calc(x * 2)}","x":["abc","1"]}`); got == "NaN" {
+			return
+		}
 	}
+	t.Fatal("calc over a sometimes non-numeric field never printed NaN in 50 draws")
 }
 
 // TestCalcReproducible pins that a calc over a random operand stays seed-stable.
@@ -218,3 +222,26 @@ func TestCalcHoldIsPerTemplate(t *testing.T) {
 		t.Fatal("the nested template never differed from its parent, want its own draw")
 	}
 }
+
+func TestCalcOverANeverNumericOperandIsRejected(t *testing.T) {
+	for src, want := range map[string]string{
+		`{"format":"{calc(x * 2)}","x":"abc"}`:           `"abc"`,
+		`{"format":"{calc(x * 2)}","x":["a","b"]}`:       `"x"`,
+		`{"format":"{calc(x + y)}","x":"1","y":"{{2}}"}`: `"{2}"`,
+	} {
+		_, err := compile(parse(t, src))
+		if err == nil || !strings.Contains(err.Error(), want) || !strings.Contains(err.Error(), "never a number") {
+			t.Errorf("compile(%s) = %v, want the operand rejected naming %s", src, err, want)
+		}
+	}
+	for _, ok := range []string{
+		`{"format":"{calc(x * 2)}","x":"3"}`,
+		`{"format":"{calc(x * 2)}","x":" 2.5 "}`,
+		`{"format":"{calc(x * 2)}","x":["1","abc"]}`,
+		`{"format":"{calc(x * 2)}","x":"{digits(2)}"}`,
+	} {
+		if _, err := compile(parse(t, ok)); err != nil {
+			t.Errorf("compile(%s) = %v, want it accepted", ok, err)
+		}
+	}
+}

cmd/fejkdata/main.go

@@ -185,9 +185,6 @@ func parseArgs(argv []string) (invocation, error) {
 
 // check rejects a flag combination that cannot run.
 func (in invocation) check() error {
-	if in.noShipped && len(in.dirs) == 0 {
-		return errors.New("--no-shipped-data needs at least one --data-path")
-	}
 	if in.list && len(in.paths) > 0 {
 		return errors.New("--list takes no path")
 	}
@@ -244,6 +241,9 @@ func run(args []string, stdout, stderr io.Writer) int {
 		return misuse(stderr, err)
 	}
 	f, err := fejkdata.New(in.options()...)
+	if errors.Is(err, fejkdata.ErrNoData) {
+		return misuse(stderr, errors.New("--no-shipped-data needs at least one --data-path"))
+	}
 	if err != nil {
 		fmt.Fprintln(stderr, err)
 		return 1

cmd/fejkdata/main_test.go

@@ -311,4 +311,8 @@ func TestRunNoShippedData(t *testing.T) {
 	if code != 0 || strings.TrimSpace(out) == "" {
 		t.Errorf("run = %d, out=%q", code, out)
 	}
+	code, _, errb = runOut("--no-shipped-data", "sv_SE.person")
+	if code != 2 || !strings.Contains(errb, "--no-shipped-data needs at least one --data-path") {
+		t.Errorf("--no-shipped-data alone = %d, %q, want misuse naming --data-path", code, errb)
+	}
 }

data.go

@@ -64,6 +64,9 @@ func loadData(sources []dataSource) (map[string]node, error) {
 	if err := checkNoCycles(root); err != nil {
 		return nil, err
 	}
+	if err := checkRepeatReach(root); err != nil {
+		return nil, err
+	}
 	if err := checkBoundLevelsHeld(root); err != nil {
 		return nil, err
 	}

fejkdata.go

@@ -16,6 +16,7 @@ import (
 	crand "crypto/rand"
 	"embed"
 	"encoding/binary"
+	"errors"
 	"fmt"
 	"io/fs"
 	"math/rand/v2"
@@ -27,6 +28,9 @@ import (
 //go:embed data
 var shippedFS embed.FS
 
+// ErrNoData is returned by New when no source is loaded at all.
+var ErrNoData = errors.New("no data: WithoutShippedData needs at least one WithDataPath or WithDataFS")
+
 // Generator generates fake data from a loaded namespace tree. Create one with [New].
 // It is safe for concurrent use; a seeded sequence is reproducible only when drawn
 // from one goroutine.
@@ -99,7 +103,7 @@ func New(opts ...Option) (*Generator, error) {
 	}
 	sources = append(sources, c.sources...)
 	if len(sources) == 0 {
-		return nil, fmt.Errorf("fejkdata: no data: WithoutShippedData needs at least one WithDataPath or WithDataFS")
+		return nil, fmt.Errorf("fejkdata: %w", ErrNoData)
 	}
 	cats, err := loadData(sources)
 	if err != nil {

loading_test.go

@@ -181,3 +181,23 @@ func TestHiddenEntriesAreSkipped(t *testing.T) {
 		}
 	}
 }
+
+func TestRepeatProductAlongAPathIsCapped(t *testing.T) {
+	for name, files := range map[string]map[string]string{
+		"nested": {"cat": `{"format":"{a}","repeat":2048,"a":{"format":"{b}","repeat":2048,"b":"x"}}`},
+		"through a reference": {
+			"a": `{"format":"{..b}","repeat":2048}`,
+			"b": `{"format":"x","repeat":2048}`,
+		},
+	} {
+		_, err := New(WithoutShippedData(), WithDataPath(writeData(t, files)))
+		if err == nil || !strings.Contains(err.Error(), "repeat") || !strings.Contains(err.Error(), "1048576") {
+			t.Errorf("%s: New = %v, want the repeat product rejected naming the maximum", name, err)
+		}
+	}
+	if _, err := New(WithoutShippedData(), WithDataPath(writeData(t, map[string]string{
+		"cat": `{"format":"{a}{c}","repeat":1024,"a":{"format":"{b}","repeat":1024,"b":"x"},"c":{"format":"y","repeat":1024}}`,
+	}))); err != nil {
+		t.Errorf("New = %v, want 1024 x 1024 along one path accepted", err)
+	}
+}

node.go

@@ -144,8 +144,8 @@ func compileChoice(items []any) (node, error) {
 		c.items[i] = n
 	}
 	if weighted { // uniform choices skip the weight table and pick in O(1)
-		if total <= 0 || math.IsInf(total, 1) {
-			return nil, fmt.Errorf("choice weights must sum to a finite positive number, got %v", total)
+		if math.IsInf(total, 1) {
+			return nil, fmt.Errorf("choice weights must sum to a finite number, got %v", total)
 		}
 		c.cum = cum
 	}
@@ -293,7 +293,7 @@ func repeatOf(m map[string]any) (int, error) {
 }
 
 // weightOf reads a node's "weight" (default 1) from its raw JSON form. Only
-// template objects carry weight; a present one must be finite and non-negative.
+// template objects carry weight; a present one must be finite and positive.
 func weightOf(raw any) (float64, error) {
 	m, ok := raw.(map[string]any)
 	if !ok {
@@ -308,7 +308,10 @@ func weightOf(raw any) (float64, error) {
 		return 0, fmt.Errorf("weight must be a number, got %T", wv)
 	}
 	if w < 0 || math.IsNaN(w) || math.IsInf(w, 0) {
-		return 0, fmt.Errorf("weight must be finite and non-negative, got %v", w)
+		return 0, fmt.Errorf("weight must be finite and positive, got %v", w)
+	}
+	if w == 0 {
+		return 0, fmt.Errorf("weight 0 means the item is never drawn; remove the item instead")
 	}
 	if w == 1 {
 		return 0, fmt.Errorf("weight 1 is the default, so it has no effect; drop it")

reference.go

@@ -412,6 +412,36 @@ func pathLeaves(n node, tail []string) []node {
 	return pathLeaves(child(n, tail[0]), tail[1:])
 }
 
+// checkRepeatReach bounds the renders a repeat multiplies to along any root-to-leaf
+// path, so nested repeats cannot build what one repeat may not. It runs after
+// checkNoCycles, whose guarantee is what lets the walk terminate.
+func checkRepeatReach(root map[string]node) error {
+	reach := map[node]int{}
+	var of func(n node) int
+	of = func(n node) int {
+		if r, done := reach[n]; done {
+			return r
+		}
+		r := 1
+		for _, e := range renderEdges(n) {
+			if c := of(e.to); c > r {
+				r = c
+			}
+		}
+		if t, ok := n.(*template); ok {
+			r *= t.repeat
+		}
+		reach[n] = r
+		return r
+	}
+	return walkNodes(root, func(path string, n node) error {
+		if t, ok := n.(*template); ok && t.repeat > 1 && of(n) > maxLen {
+			return fmt.Errorf("%s: repeat %d multiplies to %d renders along one path, above the maximum %d", path, t.repeat, of(n), maxLen)
+		}
+		return nil
+	})
+}
+
 // checkNoCycles rejects a reference cycle: a node whose rendering can reach itself
 // — directly, mutually, or through a chain — never terminates, so it must fail at
 // New rather than stack-overflow at render. It is a depth-first walk of the render

shipped_data_test.go

@@ -1,6 +1,7 @@
 package fejkdata
 
 import (
+	"errors"
 	"reflect"
 	"slices"
 	"strings"
@@ -55,8 +56,8 @@ func TestUserDataMayReferenceShipped(t *testing.T) {
 
 func TestWithoutShippedDataNeedsASource(t *testing.T) {
 	_, err := New(WithoutShippedData())
-	if err == nil || !strings.Contains(err.Error(), "WithDataPath") {
-		t.Fatalf("New(WithoutShippedData()) = %v, want an error naming WithDataPath", err)
+	if err == nil || !strings.Contains(err.Error(), "WithDataPath") || !errors.Is(err, ErrNoData) {
+		t.Fatalf("New(WithoutShippedData()) = %v, want ErrNoData naming WithDataPath", err)
 	}
 }
 

template_test.go

@@ -121,12 +121,10 @@ func TestAlternationPicksOneField(t *testing.T) {
 	}
 }
 
-func TestWeightZeroNeverChosen(t *testing.T) {
-	f := engine(5)
-	for i := 0; i < 200; i++ {
-		if got := mustRender(t, f, `[{"format":"X","weight":0},"Y"]`); got != "Y" {
-			t.Fatalf("weight-0 variant chosen: %q", got)
-		}
+func TestWeightZeroIsRejected(t *testing.T) {
+	_, err := compile(parse(t, `[{"format":"X","weight":0},"Y"]`))
+	if err == nil || !strings.Contains(err.Error(), "weight 0") || !strings.Contains(err.Error(), "remove") {
+		t.Fatalf("compile(weight 0) = %v, want it rejected naming the fix", err)
 	}
 }