Fence a record once per node, refuse a column reading its own record, and keep a bare reference independent as an operand
Tests / vet + fmt + tests (pull_request) Successful in 1m3s

This commit is contained in:
2026-09-04 09:32:29 +02:00
parent 78572ccbbc
commit 79827a66d6
6 changed files with 100 additions and 33 deletions
+8 -6
View File
@@ -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}`
is refused, naming the fields to write instead, exactly as
[One draw, one spelling](#one-draw-one-spelling) refuses the pair inside a single
format. A field hold, transform or operand ties fields together within one column
as always (see [Correlated fields](#correlated-fields) and
[Decisions](#decisions)).
columns may not read overlapping reference *paths*`{/cat.a}` beside
`{/cat.a.b}` is refused, naming the fields to write instead, as
[One draw, one spelling](#one-draw-one-spelling) refuses that pair inside a single
format. A column may not reference the record it belongs to either: `{/users.first}`
inside `users` would contradict the `first` column beside it, so put a value two
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
+1
View File
@@ -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()}
+3 -3
View File
@@ -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
// sibling stays local to it.
// A reference that reads a path reads the caller's scope, so its draw outlives
// 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 {
+19
View File
@@ -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) {
+56 -21
View File
@@ -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)
if err != nil {
return nil, fmt.Errorf("fejkdata: %s %w", path, err)
shape := f.recordShapeOf(n)
if shape.err != nil {
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
// columns: one renders a level the other reads a path into, and the record's one
// draw of that level cannot answer for both. checkNoOverlap settles the pair
// 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.
// checkColumnRefs rejects the reference reads a record's shared draw cannot answer
// for: one column rendering a level another reads a path into, and a column
// reading the record back through its own path.
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
// the same edges expand does.
func columnRefs(t *template, columns []string) []columnRef {
// columnRefs lists every reference read that reads a path, anywhere a column
// renders, following the same edges expand does. A read that lands back on the
// 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
// columns share one reference scope, so two columns that reference one category
// read one draw of it.
// renderRecord draws each column once, in the name order recordOf fixed, over one
// reference scope shared across them.
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))}
+13 -3
View File
@@ -152,9 +152,9 @@ func TestRecordRejectsOverlappingReferenceColumns(t *testing.T) {
cat := `{"format":"","a":[{"format":"A={b}","b":"1"},{"format":"A={b}","b":"2"}]}`
for _, c := range []struct{ name, row string }{
{"sibling columns", `{"format":"","whole":"{/cat.a}","inner":"{/cat.a.b}"}`},
{"through a nested template", `{"format":"","whole":"{/cat.a}","inner":{"format":"{/cat.a.b}"}}`},
{"through a nested template", `{"format":"","whole":"{/cat.a}","inner":{"format":"{/cat.a.b} {x}","x":"1"}}`},
{"through a column repeat", `{"format":"","whole":"{/cat.a}","inner":{"format":"{/cat.a.b}","repeat":2,"separator":"-"}}`},
{"through a choice variant", `{"format":"","whole":"{/cat.a}","inner":[{"format":"{/cat.a.b}"},{"format":"{/cat.a.b}!"}]}`},
{"through a choice variant", `{"format":"","whole":"{/cat.a}","inner":[{"format":"{/cat.a.b} {x}","x":"1"},{"format":"{/cat.a.b}! {x}","x":"2"}]}`},
{"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))
@@ -172,6 +172,17 @@ func TestRecordRejectsOverlappingReferenceColumns(t *testing.T) {
}
}
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}"}`,
@@ -361,7 +372,6 @@ func TestInlineRecordErrors(t *testing.T) {
{`"hello"`, "has no fields, so no columns"},
{`["a","b"]`, "a record is a template whose fields are its columns"},
{`{"format":"{a}-","repeat":3,"separator":"|","a":["x","y"]}`, "carries repeat 3"},
{`{"format":"","whole":"{/cur.code}","inner":"{/cur.code.x}"}`, "reads a path into"},
} {
if _, err := f.FakeRecord(c.input); err == nil || !strings.Contains(err.Error(), c.want) {
t.Errorf("FakeRecord(%q) = %v, want an error naming %q", c.input, err, c.want)