diff --git a/bound_test.go b/bound_test.go index cda5061..682d4d1 100644 --- a/bound_test.go +++ b/bound_test.go @@ -93,11 +93,22 @@ func TestReferenceNamingABoundLevelIsRejected(t *testing.T) { // A reference can name a bound level from the data root, which renders it // afresh beside the path that reads its held draw — the same overlap by // another spelling. - _, err := New([]string{writeData(t, map[string]string{ - "cat": `{"format":"{p.first}|{..cat.p}","p":[{"format":"{first}","first":["Anna","Bo"]}]}`, - })}) - if err == nil || !strings.Contains(err.Error(), "reads a path into") { - t.Fatalf("New = %v, want the reference rejected as an overlap", err) + rejected := map[string]string{ + "reference names the head": `{"format":"{p.first}|{..cat.p}","p":[{"format":"{first}","first":["Anna","Bo"]}]}`, + "reference names the leaf": `{"format":"{p.addr}|{..cat.p.addr}","p":{"format":"x","addr":["A","B","C","D"]}}`, + } + for name, file := range rejected { + _, err := New([]string{writeData(t, map[string]string{"cat": file})}) + if err == nil || !strings.Contains(err.Error(), "reads a path into") { + t.Errorf("%s: New = %v, want the reference rejected as an overlap", name, err) + } + } + // A reference to anything this format does not bind is untouched. + if _, err := New([]string{writeData(t, map[string]string{ + "cat": `{"format":"{p.first} {..surname}","p":[{"format":"{first}","first":["Anna","Bo"]}]}`, + "surname": `["Eriksson","Lindqvist"]`, + })}); err != nil { + t.Errorf("New = %v, want a reference outside the bound level accepted", err) } } diff --git a/node.go b/node.go index a4c050f..e795062 100644 --- a/node.go +++ b/node.go @@ -49,7 +49,9 @@ type template struct { // bound is the fields the format addresses by dotted path, each drawn once per // expansion so two tokens read one row (see compileOps and expand). nil when // the format takes no path. - bound map[string]bool + // Each maps the field to one path token reading it, for naming the other half + // of an overlap. + bound map[string]string } func (*template) isNode() {} @@ -161,7 +163,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); err != nil { + if err := checkNoOverlap(t.ops, t.bound, format); err != nil { return nil, err } return t, nil @@ -171,28 +173,35 @@ func compileTemplate(m map[string]any) (node, error) { // address a node whichever way the draw goes. A multi-variant choice must carry the // whole remaining path in the set every variant shares, so a path that validates // here resolves on every render, and a typo is a New-time error. -func checkPath(n node, tail []string) error { +func checkPath(n node, tail []string, level string) error { if len(tail) == 0 { return nil } switch n := n.(type) { case *template: if n.repeat > 1 { - return fmt.Errorf("it carries a repeat, which a path reading one draw of it cannot apply") + return fmt.Errorf("the level %q carries a repeat, which a path reading one draw of it cannot apply", level) } child, ok := n.fields[tail[0]] if !ok { return fmt.Errorf("no field %q", tail[0]) } - return checkPath(child, tail[1:]) + return checkPath(child, tail[1:], level+"."+tail[0]) case *choice: if len(n.items) > 1 { if want := strings.Join(tail, "."); !n.shared[want] { return unreachableInChoice(n, want) } - return nil // every variant carries it, so any draw resolves + // Reachability is settled; each variant still answers for itself, so a + // rule about the level (its repeat) holds behind a choice as in front. + for _, item := range n.items { + if err := checkPath(item, tail, level); err != nil { + return err + } + } + return nil } - return checkPath(n.items[0], tail) + return checkPath(n.items[0], tail, level) case literal: return fmt.Errorf("a plain string has no field %q", tail[0]) default: diff --git a/reference.go b/reference.go index f0ec3cf..23fb7f5 100644 --- a/reference.go +++ b/reference.go @@ -32,12 +32,49 @@ func linkRefs(root map[string]node) error { if err != nil { return fmt.Errorf("%s: reference {%s}: %w", path, name, err) } + if err := checkNotBound(t, name, target); err != nil { + return fmt.Errorf("%s: %w", path, err) + } t.fields[name] = target } return nil }) } +// checkNotBound rejects a reference that lands on a level this format binds, or +// anywhere under one. Rendering the target draws it afresh beside the path that +// reads its held draw, which is the overlap checkNoOverlap rejects between tokens +// — a reference is one more spelling of it, and the only one that can reach a +// level from outside the format. Heads are checked in sorted order so which +// overlap is reported does not vary. +func checkNotBound(t *template, ref string, target node) error { + heads := make([]string, 0, len(t.bound)) + for head := range t.bound { + heads = append(heads, head) + } + sort.Strings(heads) + for _, head := range heads { + if reaches(t.fields[head], target) { + return fmt.Errorf("reference {%s} renders a level that {%s} reads a path into; name the fields you want instead", ref, t.bound[head]) + } + } + return nil +} + +// reaches reports whether want is n or a node contained in it. Containment is the +// JSON structure, so the walk is over a tree and always terminates. +func reaches(n, want node) bool { + if n == want { + return true + } + for _, c := range contained(n) { + if reaches(c.node, want) { + return true + } + } + return false +} + // walkNodes calls fn once per contained node, passing the dot path that reaches it, // visiting keys in sorted order so which of several broken nodes gets reported does // not depend on map iteration. diff --git a/render.go b/render.go index 3f32ac8..74286d9 100644 --- a/render.go +++ b/render.go @@ -178,7 +178,7 @@ type draws struct { // linkRefs prove every step, so this cannot fail. func resolve(s *session, arms []arm, t *template, bound *draws) string { a := arms[s.IntN(len(arms))] - if !t.bound[a.key] { + if _, isBound := t.bound[a.key]; !isBound { return render(s, t.fields[a.key]) } if v, read := bound.value[a.name]; read { @@ -190,8 +190,9 @@ func resolve(s *session, arms []arm, t *template, bound *draws) string { bound.variant[a.key] = n } // Hold the draw at every level passed through, so two paths sharing a prefix - // share it; the leaf needs no hold, as checkNoOverlap means no other token - // can name it. + // share it. The leaf needs no hold: no other token can name it, since + // checkNoOverlap rejects a field or calc spelling and checkNotBound a + // reference one. for i, seg := range a.tail { if i < len(a.steps) { held, drew := bound.variant[a.steps[i]] diff --git a/template.go b/template.go index b6d9922..64355cc 100644 --- a/template.go +++ b/template.go @@ -154,7 +154,7 @@ func checkTokens(format string, fields map[string]node) error { } return fmt.Errorf("token {%s}: no field %q", t.body, a.key) } - if err := checkPath(head, a.tail); err != nil { + if err := checkPath(head, a.tail, a.key); err != nil { return fmt.Errorf("token {%s}: field %q: %w", t.body, a.key, err) } } @@ -211,29 +211,39 @@ 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]bool) error { - var names []string +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 bound[a.key] { - names = append(names, a.name) + if _, isBound := bound[a.key]; isBound { + names = append(names, reader{a.name, "token {" + a.name + "}"}) } } } - sort.Strings(names) + // 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)}) + } + } + sort.Slice(names, func(i, j int) bool { return names[i].name < names[j].name }) for i, level := range names { for _, path := range names[i+1:] { - if strings.HasPrefix(path, level+".") { - return fmt.Errorf("token {%s} renders a level that {%s} reads a path into; name the fields you want instead", level, path) + if strings.HasPrefix(path.name, level.name+".") { + return fmt.Errorf("%s renders a level that {%s} reads a path into; name the fields you want instead", level.label, path.name) } } } return nil } +// reader is one way a format reaches a bound field: the name it reads, and how to +// name that spelling in an error. +type reader struct{ name, label string } + // 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. @@ -282,10 +292,10 @@ type op struct { // so every token reading one sees the same row (see expand). bound is nil when the // format takes no path, so data that uses none carries no render-time cost. // Call checkTokens first: it is what proves the scan and every token are valid. -func compileOps(format string) ([]op, int, map[string]bool) { +func compileOps(format string) ([]op, int, map[string]string) { var ops []op var lit strings.Builder - var bound map[string]bool + var bound map[string]string grow := 0 flush := func() { if lit.Len() > 0 { @@ -311,9 +321,11 @@ func compileOps(format string) ([]op, int, map[string]bool) { for _, a := range arms { if len(a.tail) > 0 { if bound == nil { - bound = map[string]bool{} + bound = map[string]string{} + } + if _, named := bound[a.key]; !named { + bound[a.key] = a.name // the first path reading it, for error messages } - bound[a.key] = true } } ops = append(ops, op{kind: 'f', arms: arms})