From 5a69b02ee00809e8e8bc1dcfbbb99f498a8a697a Mon Sep 17 00:00:00 2001 From: M Date: Sun, 30 Aug 2026 22:27:56 +0200 Subject: [PATCH] Find a format's bound readers in one ordered scan --- bound_test.go | 12 ++++++++++++ calc.go | 24 +++++++++++++++++------- calc_test.go | 15 +++++++++++++++ node.go | 2 +- template.go | 51 +++++++++++++++++++++++++++++++-------------------- 5 files changed, 76 insertions(+), 28 deletions(-) diff --git a/bound_test.go b/bound_test.go index 4cd2294..5207fa9 100644 --- a/bound_test.go +++ b/bound_test.go @@ -214,6 +214,18 @@ func TestASharedNodeIsWalkedOnce(t *testing.T) { } } +func TestTheEarlierReaderIsNamed(t *testing.T) { + // A token and a calc operand can name one level. The one the format writes + // first is the one reported, so the error points at the same place a reader + // looking at the format would start. + _, err := New([]string{writeData(t, map[string]string{ + "cat": `{"format":"{calc(p * 1)} {p} {p.first}","p":[{"format":"{first}","first":["1","2"]}]}`, + })}) + if err == nil || !strings.Contains(err.Error(), `calc operand "p"`) { + t.Fatalf("New = %v, want the calc operand named, being written first", err) + } +} + func TestReferenceToAMatchingStringIsAccepted(t *testing.T) { // A literal renders one fixed string, so no draw of it can disagree with a // held one. Two unrelated literals that merely spell the same text must not diff --git a/calc.go b/calc.go index 324c824..bffada5 100644 --- a/calc.go +++ b/calc.go @@ -101,19 +101,29 @@ func calcPrep(args []string) callFn { func calcOperands(format string) []string { var names []string _ = eachToken(format, func(t ftoken) error { - if t.kind != 'b' { - return nil - } - if name, args, ok := funcCall(t.body); ok && name == "calc" && len(args) >= 1 { - if expr, err := parseCalc(args[0]); err == nil { - names = append(names, calcVars(expr)...) - } + if t.kind == 'b' { + names = append(names, calcTokenOperands(t.body)...) } return nil }) return names } +// calcTokenOperands lists the sibling-field names one {token} body reads, empty +// for anything that is not a {calc(...)}. checkCalc reports an expression that does +// not parse, so one that does not simply names nothing here. +func calcTokenOperands(body string) []string { + name, args, ok := funcCall(body) + if !ok || name != "calc" || len(args) == 0 { + return nil + } + expr, err := parseCalc(args[0]) + if err != nil { + return nil + } + return calcVars(expr) +} + // calcVars lists the field names an expression references, for the existence // check in checkCalc. func calcVars(n calcNode) []string { diff --git a/calc_test.go b/calc_test.go index f62d48e..351a389 100644 --- a/calc_test.go +++ b/calc_test.go @@ -82,3 +82,18 @@ func TestCalcReproducible(t *testing.T) { t.Fatalf("calc not reproducible: %q != %q", a, b) } } + +// TestCalcTokenOperandsReadsOnlyACalc pins what the helper answers for a body that +// is not a calc, and for one whose expression does not parse: nothing, either way. +// checkCalc is what reports a bad expression, so the callers that run after it +// never meet one — but they must not have to depend on that order to be safe. +func TestCalcTokenOperandsReadsOnlyACalc(t *testing.T) { + for _, body := range []string{"plain", "luhn()", "calc()", "calc(1 +)", "calc(()"} { + if got := calcTokenOperands(body); got != nil { + t.Errorf("calcTokenOperands(%q) = %v, want none", body, got) + } + } + if got := calcTokenOperands("calc(net * qty)"); len(got) != 2 { + t.Errorf("calcTokenOperands(calc(net * qty)) = %v, want both operands", got) + } +} diff --git a/node.go b/node.go index 21041d3..baad6d0 100644 --- a/node.go +++ b/node.go @@ -161,7 +161,7 @@ func compileTemplate(m map[string]any) (node, error) { return nil, err } t.ops, t.grow, t.bound = compileOps(format) - if err := checkNoOverlap(t.ops, t.bound, format); err != nil { + if err := checkNoOverlap(format, t.bound); err != nil { return nil, err } return t, nil diff --git a/template.go b/template.go index 0792f5e..1d660a2 100644 --- a/template.go +++ b/template.go @@ -211,26 +211,10 @@ func splitArm(name string) arm { // expands it afresh, so their values disagree. One spelling, so there is nothing // to get wrong. Names are compared in sorted order, so which pair is reported // does not depend on where the tokens sit. -func checkNoOverlap(ops []op, bound map[string]string, format string) error { - var names []reader - for i := range ops { - if ops[i].kind != 'f' { - continue - } - for _, a := range ops[i].arms { - if _, isBound := bound[a.key]; isBound { - names = append(names, reader{a.name, "token {" + a.name + "}"}) - } - } - } - // A calc operand renders its field, so it names a level exactly as a token does. - for _, name := range calcOperands(format) { - if _, isBound := bound[name]; isBound { - names = append(names, reader{name, fmt.Sprintf("calc operand %q", name)}) - } - } - // Stable, so two readers of one name (a token and a calc operand both naming - // "p") are reported in the order the format writes them. +func checkNoOverlap(format string, bound map[string]string) error { + names := boundReaders(format, bound) + // Stable over one format-order scan, so two readers of one name (a token and a + // calc operand both naming "p") are reported as the format writes them. sort.SliceStable(names, func(i, j int) bool { return names[i].name < names[j].name }) for i, level := range names { for _, path := range names[i+1:] { @@ -245,6 +229,33 @@ func checkNoOverlap(ops []op, bound map[string]string, format string) error { // reader is one way a format reaches a bound field, and how to name that spelling. type reader struct{ name, label string } +// boundReaders lists every way a format reaches a bound field, in the order the +// format writes them. A calc operand renders its field, so it names a level exactly +// as a token does; one scan finds both, which is what puts them in one order. +func boundReaders(format string, bound map[string]string) []reader { + var names []reader + _ = eachToken(format, func(t ftoken) error { + if t.kind != 'b' { + return nil + } + if _, _, isFunc := funcCall(t.body); isFunc { + for _, operand := range calcTokenOperands(t.body) { + if _, isBound := bound[operand]; isBound { + names = append(names, reader{operand, fmt.Sprintf("calc operand %q", operand)}) + } + } + return nil + } + for _, a := range splitArms(t.body) { + if _, isBound := bound[a.key]; isBound { + names = append(names, reader{a.name, "token {" + a.name + "}"}) + } + } + return nil + }) + return names +} + // checkSegments rejects an unfinished path: "{a.}", "{.b}" and "{a..b}" each have // a segment naming nothing. A field really named "" would otherwise make them // resolve, so a typo would read as a path that worked.