Fence a record once per node, refuse a column reading its own record, and keep a bare reference independent as an operand

README.md

@@ -126,12 +126,14 @@ renders nothing by `Fake`, and is compiled only so the tree's fences still run.
 The columns are the point, and their facts stay together: two columns that
 reference one category — `{/currency.code}` and `{/currency.symbol}` — share one
 draw of it, so the record is internally consistent. That one draw is also why two
-columns may not read overlapping reference paths — `{/cat.a}` beside `{/cat.a.b}`
+columns may not read overlapping reference *paths* — `{/cat.a}` beside
-is refused, naming the fields to write instead, exactly as
+`{/cat.a.b}` is refused, naming the fields to write instead, as
-[One draw, one spelling](#one-draw-one-spelling) refuses the pair inside a single
+[One draw, one spelling](#one-draw-one-spelling) refuses that pair inside a single
-format. A field hold, transform or operand ties fields together within one column
+format. A column may not reference the record it belongs to either: `{/users.first}`
-as always (see [Correlated fields](#correlated-fields) and
+inside `users` would contradict the `first` column beside it, so put a value two
-[Decisions](#decisions)).
+columns share in its own category and reference that. A field hold, transform or
+operand ties fields together within one column as always (see
+[Correlated fields](#correlated-fields) and [Decisions](#decisions)).
 
 ## Library
 

fejkdata.go

@@ -45,6 +45,7 @@ type Generator struct {
 	mu         sync.Mutex
 	rand       *session
 	categories map[string]node
+	records    map[node]recordShape
 }
 
 // session is one generator's mutable render state: the seeded rng plus the {seq()}

hold.go

@@ -274,10 +274,10 @@ func readField(s *session, t *template, held, refScope *draws, a arm) string {
 		}
 		return render(s, t.fields[a.key], refScope)
 	}
-	// A reference reads the caller's scope, so its draw outlives this expansion; a
+	// A reference that reads a path reads the caller's scope, so its draw outlives
-	// sibling stays local to it.
+	// this expansion; a sibling, and a reference read whole, stay local to it.
 	d := held
-	if isRef(a.key) && refScope != nil {
+	if isRef(a.key) && refScope != nil && len(a.tail) > 0 {
 		d = refScope
 	}
 	if v, read := d.value[a.path]; read {

perf_test.go

@@ -51,6 +51,25 @@ func TestNoRenderAllocRegression(t *testing.T) {
 	}
 }
 
+// A record's fences read the compiled tree, so they belong to New, not to a draw.
+// A per-draw walk costs allocations in proportion to the tree; this pins that the
+// count does not move with the column count.
+func TestNoRecordAllocRegression(t *testing.T) {
+	for _, s := range []struct{ name, json string }{
+		{"record 3 columns", `{"format":"","a":"x","b":"y","c":"z"}`},
+		{"record 50 columns", wideTokenJSON(50)},
+	} {
+		f, err := New(WithoutShippedData(), WithDataFS(fstest.MapFS{"x.json": {Data: []byte(s.json)}}))
+		if err != nil {
+			t.Fatalf("New(%s): %v", s.name, err)
+		}
+		const base = 4.0
+		if allocs := testing.AllocsPerRun(10000, func() { f.Record("x") }); allocs > base*1.10 {
+			t.Errorf("%s: %.1f allocs/op regressed past %.1f (baseline %.1f + 10%%); a record fence running per draw is the usual cause", s.name, allocs, base*1.10, base)
+		}
+	}
+}
+
 func BenchmarkNestedDepth25(b *testing.B)  { benchPath(b, tmpData(b, "deep", nestedJSON(25)), "deep") }
 func BenchmarkNestedDepth100(b *testing.B) { benchPath(b, tmpData(b, "deep", nestedJSON(100)), "deep") }
 func BenchmarkWideTokens100(b *testing.B) {

record.go

@@ -17,7 +17,8 @@ type Column struct {
 
 // Record is one record rendered from a template: every direct field is a column,
 // listed in name order. Each column is its own expansion, so a sibling field is
-// local to it, while a reference is drawn once for the whole record.
+// local to it, while a reference that reads a path is drawn once for the whole
+// record.
 type Record struct {
 	columns []Column
 }
@@ -104,11 +105,35 @@ func (f *Generator) Record(path string) (*Record, error) {
 	if len(tail) > 0 {
 		return nil, fmt.Errorf("fejkdata: %s descends into %q, a field; only a category-level template is a record", path, tail[0])
 	}
-	t, columns, err := recordOf(n)
+	shape := f.recordShapeOf(n)
-	if err != nil {
+	if shape.err != nil {
-		return nil, fmt.Errorf("fejkdata: %s %w", path, err)
+		return nil, fmt.Errorf("fejkdata: %s %w", path, shape.err)
 	}
-	return renderRecord(f.rand, t, columns), nil
+	return renderRecord(f.rand, shape.t, shape.columns), nil
+}
+
+// recordShape is what recordOf settled about a node: the template to project, its
+// columns, or why it is not a record.
+type recordShape struct {
+	t       *template
+	columns []string
+	err     error
+}
+
+// recordShapeOf fences a node once and remembers the answer. The fences read the
+// compiled tree, which New fixed, so a repeated Record call on one path pays them
+// once rather than per draw. Callers hold the generator's lock.
+func (f *Generator) recordShapeOf(n node) recordShape {
+	if shape, done := f.records[n]; done {
+		return shape
+	}
+	t, columns, err := recordOf(n)
+	shape := recordShape{t: t, columns: columns, err: err}
+	if f.records == nil {
+		f.records = map[node]recordShape{}
+	}
+	f.records[n] = shape
+	return shape
 }
 
 // RecordTemplate is an inline record compiled, referenced and validated once,
@@ -169,14 +194,14 @@ func recordOf(n node) (*template, []string, error) {
 	return t, columns, nil
 }
 
-// checkColumnRefs rejects two reference reads that overlap across a record's
+// checkColumnRefs rejects the reference reads a record's shared draw cannot answer
-// columns: one renders a level the other reads a path into, and the record's one
+// for: one column rendering a level another reads a path into, and a column
-// draw of that level cannot answer for both. checkNoOverlap settles the pair
+// reading the record back through its own path.
-// within a single format; the record's shared scope is what carries it across
-// columns, so the same pair is settled here. A bare reference holds nothing, so it
-// is not collected and keeps drawing on its own.
 func checkColumnRefs(t *template, columns []string) error {
-	reads := columnRefs(t, columns)
+	reads, err := columnRefs(t, columns)
+	if err != nil {
+		return err
+	}
 	sort.Slice(reads, func(i, j int) bool {
 		if reads[i].a.path != reads[j].a.path {
 			return reads[i].a.path < reads[j].a.path
@@ -200,21 +225,32 @@ type columnRef struct {
 	a      arm
 }
 
-// columnRefs lists every held reference read anywhere a column renders, following
+// columnRefs lists every reference read that reads a path, anywhere a column
-// the same edges expand does.
+// renders, following the same edges expand does. A read that lands back on the
-func columnRefs(t *template, columns []string) []columnRef {
+// record itself names a sibling column, which no draw of the record can answer
+// for, so it is reported here rather than collected.
+func columnRefs(t *template, columns []string) ([]columnRef, error) {
 	var out []columnRef
+	var err error
 	for _, name := range columns {
 		seen := map[node]bool{}
 		var walk func(n node)
 		walk = func(n node) {
-			if n == nil || seen[n] {
+			if n == nil || seen[n] || err != nil {
 				return
 			}
 			seen[n] = true
 			if tm, ok := n.(*template); ok {
 				for _, r := range boundReaders(tm.format, tm.bound, tm.refs) {
-					if a := splitArm(r.name, tm.refs); isRef(a.key) && len(a.tail) > 0 {
+					a := splitArm(r.name, tm.refs)
+					if !isRef(a.key) {
+						continue
+					}
+					if tm.fields[a.key] == node(t) {
+						err = fmt.Errorf("column %q reads {%s}, which points back at this record; a column cannot read another column — move the shared value into its own category and reference that", name, a.name)
+						return
+					}
+					if len(a.tail) > 0 {
 						out = append(out, columnRef{name, a})
 					}
 				}
@@ -225,12 +261,11 @@ func columnRefs(t *template, columns []string) []columnRef {
 		}
 		walk(t.fields[name])
 	}
-	return out
+	return out, err
 }
 
-// renderRecord draws each column once, in the name order recordOf fixed. The
+// renderRecord draws each column once, in the name order recordOf fixed, over one
-// columns share one reference scope, so two columns that reference one category
+// reference scope shared across them.
-// read one draw of it.
 func renderRecord(s *session, t *template, columns []string) *Record {
 	scope := &draws{variant: map[string]node{}, value: map[string]string{}}
 	r := &Record{columns: make([]Column, len(columns))}

record_test.go

@@ -149,16 +149,79 @@ func TestRecordCSVEmptyValueStaysARow(t *testing.T) {
 }
 
 func TestRecordRejectsOverlappingReferenceColumns(t *testing.T) {
-	dir := writeData(t, map[string]string{
+	cat := `{"format":"","a":[{"format":"A={b}","b":"1"},{"format":"A={b}","b":"2"}]}`
-		"cat": `{"format":"","a":[{"format":"A={b}","b":"1"},{"format":"A={b}","b":"2"}]}`,
+	for _, c := range []struct{ name, row string }{
-		"row": `{"format":"","whole":"{/cat.a}","inner":"{/cat.a.b}"}`,
+		{"sibling columns", `{"format":"","whole":"{/cat.a}","inner":"{/cat.a.b}"}`},
-	})
+		{"through a nested template", `{"format":"","whole":"{/cat.a}","inner":{"format":"{/cat.a.b} {x}","x":"1"}}`},
-	f := newGenerator(t, dir, WithSeed(1))
+		{"through a column repeat", `{"format":"","whole":"{/cat.a}","inner":{"format":"{/cat.a.b}","repeat":2,"separator":"-"}}`},
-	if _, err := f.Record("row"); err == nil || !strings.Contains(err.Error(), "reads a path into") {
+		{"through a choice variant", `{"format":"","whole":"{/cat.a}","inner":[{"format":"{/cat.a.b} {x}","x":"1"},{"format":"{/cat.a.b}! {x}","x":"2"}]}`},
-		t.Fatalf("Record over an overlapping reference pair = %v, want it rejected the way one format is", err)
+		{"as a builtin operand", `{"format":"","whole":"{uppercase(/cat.a)}","inner":"{/cat.a.b}"}`},
+	} {
+		f := newGenerator(t, writeData(t, map[string]string{"cat": cat, "row": c.row}), WithSeed(1))
+		_, err := f.Record("row")
+		if err == nil || !strings.Contains(err.Error(), "reads a path into") {
+			t.Errorf("%s: Record = %v, want the overlap rejected the way one format is", c.name, err)
+			continue
+		}
+		if !strings.Contains(err.Error(), `"whole"`) || !strings.Contains(err.Error(), `"inner"`) {
+			t.Errorf("%s: error %q names neither column; it must name both", c.name, err)
 		}
 		if _, err := f.Fake("row"); err != nil {
-		t.Errorf("Fake(row) = %v, want the string view untouched", err)
+			t.Errorf("%s: Fake(row) = %v, want the string view untouched", c.name, err)
+		}
+	}
+}
+
+func TestInlineRecordRejectsOverlappingColumns(t *testing.T) {
+	dir := writeData(t, map[string]string{
+		"cat": `{"format":"","a":[{"format":"A={b}","b":"1"},{"format":"A={b}","b":"2"}]}`,
+	})
+	f := newGenerator(t, dir, WithSeed(1))
+	_, err := f.FakeRecord(`{"format":"","whole":"{/cat.a}","inner":"{/cat.a.b}"}`)
+	if err == nil || !strings.Contains(err.Error(), "reads a path into") {
+		t.Fatalf("inline record over an overlapping pair = %v, want the inline entry point to refuse it too", err)
+	}
+}
+
+func TestRecordRejectsAColumnReadingItsOwnRecord(t *testing.T) {
+	dir := writeData(t, map[string]string{
+		"person": `{"format":"{first} {last}","first":["Ada","Bo"],"last":["Lovelace","Ek"],"full":"{/person.first} {/person.last}"}`,
+	})
+	f := newGenerator(t, dir, WithSeed(1))
+	if _, err := f.Record("person"); err == nil || !strings.Contains(err.Error(), "points back at this record") {
+		t.Fatalf("a column referencing its own record = %v, want it refused; it would contradict the columns it reads", err)
+	}
+}
+
+func TestRecordBareReferenceStaysIndependentAsAnOperand(t *testing.T) {
+	dir := writeData(t, map[string]string{
+		"cur": `[{"format":"{code}","code":"aud"},{"format":"{code}","code":"eur"}]`,
+		"row": `{"format":"","up":"{uppercase(/cur)}","low":"{lowercase(/cur)}"}`,
+	})
+	f := newGenerator(t, dir, WithSeed(1))
+	sawMismatch := false
+	for i := 0; i < 200 && !sawMismatch; i++ {
+		r, err := f.Record("row")
+		if err != nil {
+			t.Fatal(err)
+		}
+		m := map[string]string{}
+		for _, c := range r.Columns() {
+			m[c.Name] = c.Value
+		}
+		sawMismatch = !strings.EqualFold(m["up"], m["low"])
+	}
+	if !sawMismatch {
+		t.Error("two bare-reference operand columns never disagreed; a bare reference draws on its own, as the plain spelling does")
+	}
+	for i := 0; i < 50; i++ {
+		v, err := f.FakeTemplate("{/cur}|{uppercase(/cur)}")
+		if err != nil {
+			t.Fatal(err)
+		}
+		if parts := strings.Split(v, "|"); !strings.EqualFold(parts[0], parts[1]) {
+			t.Fatalf("one format rendered %q; within an expansion a bare reference is still one draw", v)
+		}
 	}
 }