Close the calc and reference spellings of an overlap, and reach the repeat rule behind a choice

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{
+	rejected := map[string]string{
-		"cat": `{"format":"{p.first}|{..cat.p}","p":[{"format":"{first}","first":["Anna","Bo"]}]}`,
+		"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"]}}`,
-	if err == nil || !strings.Contains(err.Error(), "reads a path into") {
+	}
-		t.Fatalf("New = %v, want the reference rejected as an overlap", err)
+	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)
 	}
 }
 

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:

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.

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
+	// share it. The leaf needs no hold: no other token can name it, since
-	// can name it.
+	// 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]]

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 {
+func checkNoOverlap(ops []op, bound map[string]string, format string) error {
-	var names []string
+	var names []reader
 	for i := range ops {
 		if ops[i].kind != 'f' {
 			continue
 		}
 		for _, a := range ops[i].arms {
-			if bound[a.key] {
+			if _, isBound := bound[a.key]; isBound {
-				names = append(names, a.name)
+				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+".") {
+			if strings.HasPrefix(path.name, level.name+".") {
-				return fmt.Errorf("token {%s} renders a level that {%s} reads a path into; name the fields you want instead", level, path)
+				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})