Compare commits

..

2 Commits

9 changed files with 195 additions and 120 deletions
+15 -6
View File
@@ -131,9 +131,10 @@ columns that read a path into one category — `{/currency.code}` and
record may not overlap — `{/cat.a}`, or a bare `{/cat}`, beside record may not overlap — `{/cat.a}`, or a bare `{/cat}`, beside
`{/cat.a.b}` is refused, naming the fields to write instead, as `{/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 [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 by any spelling: `{/users.first}` format. Both fences run at load, so a category that loads renders as either shape. A
or a bare `{/users}` inside `users` describes a draw other than the columns beside category never references itself — `{/users.first}` or a bare `{/users}` inside `users`
it, so put a value two columns share in its own category and reference that. A field hold, transform or describes a draw other than the fields beside it — so read a sibling as a path, and put
a value two fields share in its own category and reference that. A field hold, transform or
operand ties fields together within one column as always (see operand ties fields together within one column as always (see
[Correlated fields](#correlated-fields) and [Decisions](#decisions)). [Correlated fields](#correlated-fields) and [Decisions](#decisions)).
@@ -447,9 +448,9 @@ a render of its own, in no group, so it draws anew, and a [draw group](#draw-gro
draw apart. A bare reference names no field and makes its own picks each time — draw apart. A bare reference names no field and makes its own picks each time —
`{/misc.uuid} {/misc.uuid}` is two draws — while the reference paths inside what it `{/misc.uuid} {/misc.uuid}` is two draws — while the reference paths inside what it
renders still read the render's draws. Rejected at `New`: a path that is renders still read the render's draws. Rejected at `New`: a path that is
unknown, names a folder, has no folder above, or reads a field not every variant unknown, names a folder, has no folder above, reads a field not every variant
of a choice carries, and a reference that leads back to its own value, directly, of a choice carries, or names the category the reference sits in, and a reference
mutually or through a chain. that leads back to its own value, directly, mutually or through a chain.
### Draw group ### Draw group
@@ -690,6 +691,14 @@ tokens add cost in proportion to the output.
different things — an operand pins the value its own render produced and stops at a different things — an operand pins the value its own render produced and stops at a
reference, a path pins every level it passes through — so one walk would carry both reference, a path pins every level it passes through — so one walk would carry both
rules and both scopes anyway, and tell them apart at every step. rules and both scopes anyway, and tell them apart at every step.
- **A category never references itself, and a record's fences run at load.** A category
is one unit: a reference back into it — `{/users.first}` inside `users` — describes a
draw other than the fields beside it, so `New` refuses it and the sibling path stays
the one spelling for a field of one's own. A value two fields share goes in its own
category, which both reference. That settled, a record's column fences run at `New`
too, so a category that loads renders as whichever shape is asked for, and a reference
reaching back into a category through another one is refused there as the overlap it
is.
- **A record's column set is fixed before the first draw.** Only a category-level - **A record's column set is fixed before the first draw.** Only a category-level
template is a record: a path descending into a field, or naming a folder or a template is a record: a path descending into a field, or naming a folder or a
choice, errors. A tail may pass through a choice whose variants carry different choice, errors. A tail may pass through a choice whose variants carry different
+24 -16
View File
@@ -133,7 +133,7 @@ func (c *drawCheck) checkDraws(path string, n node) error {
if !ok || !c.readsPath(t) { if !ok || !c.readsPath(t) {
return nil return nil
} }
w := newDrawWalk(nil) w := newDrawWalk()
for _, e := range renderEdges(t) { for _, e := range renderEdges(t) {
w.edge(t, e, drawAt{group: t.drawGroupKey, route: drawRoute{e.reached(), e.label}}) w.edge(t, e, drawAt{group: t.drawGroupKey, route: drawRoute{e.reached(), e.label}})
} }
@@ -143,6 +143,23 @@ func (c *drawCheck) checkDraws(path string, n node) error {
return nil return nil
} }
// checkRecordDraws fences the columns of a record — a template compiled at the top without a
// repeat — so a load proves the record view of it as well as the string view.
func (c *drawCheck) checkRecordDraws(path string, n node) error {
t, ok := n.(*template)
if !ok || !t.record {
return nil
}
columns := recordColumns(t)
if len(columns) == 0 {
return nil
}
if err := checkColumnDraws(t, columns); err != nil {
return fmt.Errorf("%s: %w", path, err)
}
return nil
}
// readsPath reports whether rendering n reads a reference path, short of a repeat, which renders // readsPath reports whether rendering n reads a reference path, short of a repeat, which renders
// over draws of its own. // over draws of its own.
func (c *drawCheck) readsPath(n node) bool { func (c *drawCheck) readsPath(n node) bool {
@@ -206,7 +223,7 @@ func refRead(n node, label string) (arm, node, bool) {
// checkColumnDraws fences a record's columns as one render. // checkColumnDraws fences a record's columns as one render.
func checkColumnDraws(t *template, columns []string) error { func checkColumnDraws(t *template, columns []string) error {
w := newDrawWalk(t) w := newDrawWalk()
for _, name := range columns { for _, name := range columns {
w.walk(t.fields[name], drawAt{group: t.drawGroupKey, route: drawRoute{spelling: fmt.Sprintf("column %q", name)}}) w.walk(t.fields[name], drawAt{group: t.drawGroupKey, route: drawRoute{spelling: fmt.Sprintf("column %q", name)}})
} }
@@ -214,14 +231,12 @@ func checkColumnDraws(t *template, columns []string) error {
} }
// drawWalk gathers what one render reads by reference and what it draws afresh, each by draw group, // drawWalk gathers what one render reads by reference and what it draws afresh, each by draw group,
// for check to compare. record is set for a record's columns, which may not read the record back. // for check to compare.
type drawWalk struct { type drawWalk struct {
record *template
reads []pathRead reads []pathRead
read map[drawKey]bool read map[drawKey]bool
fresh []freshDraw fresh []freshDraw
seen map[drawVisit]bool seen map[drawVisit]bool
err error
} }
// drawAt is where a walk stands: the draw group it draws in, how the render's root reached it, the // drawAt is where a walk stands: the draw group it draws in, how the render's root reached it, the
@@ -259,15 +274,15 @@ type drawKey struct {
key any key any
} }
func newDrawWalk(record *template) *drawWalk { func newDrawWalk() *drawWalk {
return &drawWalk{record: record, read: map[drawKey]bool{}, seen: map[drawVisit]bool{}} return &drawWalk{read: map[drawKey]bool{}, seen: map[drawVisit]bool{}}
} }
// walk follows what rendering n renders. A repeat renders over draws of its own, so the walk stops // walk follows what rendering n renders. A repeat renders over draws of its own, so the walk stops
// there. // there.
func (w *drawWalk) walk(n node, at drawAt) { func (w *drawWalk) walk(n node, at drawAt) {
v := drawVisit{n, at.group, at.held} v := drawVisit{n, at.group, at.held}
if w.seen[v] || w.err != nil { if w.seen[v] {
return return
} }
w.seen[v] = true w.seen[v] = true
@@ -287,13 +302,9 @@ func (w *drawWalk) walk(n node, at drawAt) {
func (w *drawWalk) edge(from node, e renderEdge, at drawAt) { func (w *drawWalk) edge(from node, e renderEdge, at drawAt) {
a, target, reads := refRead(from, e.label) a, target, reads := refRead(from, e.label)
switch { if !reads {
case !reads:
w.walk(e.to, at) w.walk(e.to, at)
return return
case w.record != nil && target == node(w.record):
w.err = fmt.Errorf("%s 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", at.route.spelling, a.name)
return
} }
if k := (drawKey{at.group, a.path}); !w.read[k] { if k := (drawKey{at.group, a.path}); !w.read[k] {
w.read[k] = true w.read[k] = true
@@ -306,9 +317,6 @@ func (w *drawWalk) edge(from node, e renderEdge, at drawAt) {
// check refuses what one draw per reference path cannot answer for, reads compared in path order so // check refuses what one draw per reference path cannot answer for, reads compared in path order so
// which pair is reported does not vary. // which pair is reported does not vary.
func (w *drawWalk) check() error { func (w *drawWalk) check() error {
if w.err != nil {
return w.err
}
sort.SliceStable(w.reads, func(i, j int) bool { sort.SliceStable(w.reads, func(i, j int) bool {
if w.reads[i].at.group != w.reads[j].at.group { if w.reads[i].at.group != w.reads[j].at.group {
return w.reads[i].at.group < w.reads[j].at.group return w.reads[i].at.group < w.reads[j].at.group
+3
View File
@@ -203,6 +203,9 @@ func checkRenders(s nodeScope) error {
if err := s(fence.checkDraws); err != nil { if err := s(fence.checkDraws); err != nil {
return err return err
} }
if err := s(fence.checkRecordDraws); err != nil {
return err
}
return s((&valueProof{}).checkDatatype) return s((&valueProof{}).checkDatatype)
} }
+74 -33
View File
@@ -91,17 +91,28 @@ func TestReferenceNamingABoundLevelIsRejected(t *testing.T) {
// A reference can name a bound level from the data root, which renders it // 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 // afresh beside the path that reads its held draw — the same overlap by
// another spelling. // another spelling.
rejected := map[string]string{ // A category never references itself, so each reference back into cat sits in a
"reference names the head": `{"format":"{p.first}|{/cat.p}","p":{"format":"{first}","first":["Anna","Bo"]}}`, // second category cat renders.
"reference names the leaf": `{"format":"{p.addr}|{/cat.p.addr}","p":{"format":"x","addr":["A","B","C","D"]}}`, rejected := map[string]map[string]string{
"reference names the head": {
"cat": `{"format":"{p.first}|{/hop}","p":{"format":"{first}","first":["Anna","Bo"]}}`,
"hop": `"{/cat.p}"`,
},
"reference names the leaf": {
"cat": `{"format":"{p.addr}|{/hop}","p":{"format":"x","addr":["A","B","C","D"]}}`,
"hop": `"{/cat.p.addr}"`,
},
// The reference need not sit in the format that binds: any field it renders // The reference need not sit in the format that binds: any field it renders
// reaches the level just the same, however deep. // reaches the level just the same, however deep.
"reference from a sibling field": `{"format":"{p.first}|{inner}","p":[` + "reference from a sibling field": {
"cat": `{"format":"{p.first}|{inner}","p":[` +
`{"format":"{first}-{last}","first":"A","last":"1"},{"format":"{first}-{last}","first":"B","last":"2"}],` + `{"format":"{first}-{last}","first":"A","last":"1"},{"format":"{first}-{last}","first":"B","last":"2"}],` +
`"inner":"{/cat.p}"}`, `"inner":"{/hop}"}`,
"hop": `"{/cat.p}"`,
},
} }
for name, file := range rejected { for name, files := range rejected {
_, err := New(WithoutShippedData(), WithDataPath(writeData(t, map[string]string{"cat": file}))) _, err := New(WithoutShippedData(), WithDataPath(writeData(t, files)))
if err == nil || !strings.Contains(err.Error(), "reads a path into") { 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) t.Errorf("%s: New = %v, want the reference rejected as an overlap", name, err)
} }
@@ -120,7 +131,8 @@ func TestCycleReachedOnlyByAPathTokenIsRejected(t *testing.T) {
// cycle walk has to follow it there. A head whose own format names nothing // cycle walk has to follow it there. A head whose own format names nothing
// would otherwise hide the cycle until render, where it is fatal. // would otherwise hide the cycle until render, where it is fatal.
_, err := New(WithoutShippedData(), WithDataPath(writeData(t, map[string]string{ _, err := New(WithoutShippedData(), WithDataPath(writeData(t, map[string]string{
"a": `{"format":"{p.x}","p":{"format":"static","x":"{/a}"}}`, "a": `{"format":"{p.x}","p":{"format":"static","x":"{/b}"}}`,
"b": `"{/a}"`,
}))) })))
if err == nil || !strings.Contains(err.Error(), "reference cycle") { if err == nil || !strings.Contains(err.Error(), "reference cycle") {
t.Fatalf("New = %v, want the cycle through {p.x} rejected", err) t.Fatalf("New = %v, want the cycle through {p.x} rejected", err)
@@ -131,8 +143,9 @@ func TestALevelRenderedOnlyByAPathTokenIsHeld(t *testing.T) {
// {p.a} renders q, so it is a route to the level {q.x} holds — even though p's // {p.a} renders q, so it is a route to the level {q.x} holds — even though p's
// own format names nothing. // own format names nothing.
_, err := New(WithoutShippedData(), WithDataPath(writeData(t, map[string]string{ _, err := New(WithoutShippedData(), WithDataPath(writeData(t, map[string]string{
"thing": `{"format":"{p.a} {q.x}","p":{"format":"static","a":"{/thing.q}"},` + "thing": `{"format":"{p.a} {q.x}","p":{"format":"static","a":"{/hop}"},` +
`"q":{"format":"{x}","x":["1","2"]}}`, `"q":{"format":"{x}","x":["1","2"]}}`,
"hop": `"{/thing.q}"`,
}))) })))
if err == nil || !strings.Contains(err.Error(), "reads a path into") { if err == nil || !strings.Contains(err.Error(), "reads a path into") {
t.Fatalf("New = %v, want the second route to q rejected", err) t.Fatalf("New = %v, want the second route to q rejected", err)
@@ -149,16 +162,20 @@ func TestACalcOperandIsHeldAgainstEveryRoute(t *testing.T) {
files map[string]string files map[string]string
want string // the route the error names, spelled as the author wrote it want string // the route the error names, spelled as the author wrote it
}{ }{
// A category never references itself, so each route back into cat sits in a
// second category cat renders.
"a reference beside the operand": { "a reference beside the operand": {
map[string]string{ map[string]string{
"cat": `{"format":"{/cat.net} x 2 = {calc(net * 2, 2)}","net":["10.00","20.00"]}`, "cat": `{"format":"{/hop} x 2 = {calc(net * 2, 2)}","net":["10.00","20.00"]}`,
"hop": `"{/cat.net}"`,
}, },
`{/cat.net} renders "net"`, `{/hop} renders "net"`,
}, },
"a reference one level down": { "a reference one level down": {
map[string]string{ map[string]string{
"cat": `{"format":"{calc(net * 2, 2)} {q}","net":["10.00","20.00"],` + "cat": `{"format":"{calc(net * 2, 2)} {q}","net":["10.00","20.00"],` +
`"q":"{/cat.net}"}`, `"q":"{/hop}"}`,
"hop": `"{/cat.net}"`,
}, },
`{q} renders "net"`, `{q} renders "net"`,
}, },
@@ -167,7 +184,8 @@ func TestACalcOperandIsHeldAgainstEveryRoute(t *testing.T) {
"a reference to an operand wrapped in a choice": { "a reference to an operand wrapped in a choice": {
map[string]string{ map[string]string{
"cat": `{"format":"{calc(n * 2, 2)} {q}","n":{"format":"{v}","v":["1","2"]},` + "cat": `{"format":"{calc(n * 2, 2)} {q}","n":{"format":"{v}","v":["1","2"]},` +
`"q":"{/cat.n}"}`, `"q":"{/hop}"}`,
"hop": `"{/cat.n}"`,
}, },
`{q} renders "n"`, `{q} renders "n"`,
}, },
@@ -176,7 +194,8 @@ func TestACalcOperandIsHeldAgainstEveryRoute(t *testing.T) {
"an operand reaching another operand": { "an operand reaching another operand": {
map[string]string{ map[string]string{
"cat": `{"format":"{calc(a + b, 0)}","a":{"format":"{x}","x":["1","2"]},` + "cat": `{"format":"{calc(a + b, 0)}","a":{"format":"{x}","x":["1","2"]},` +
`"b":"{/cat.a}"}`, `"b":"{/hop}"}`,
"hop": `"{/cat.a}"`,
}, },
`calc operand "b" renders "a"`, `calc operand "b" renders "a"`,
}, },
@@ -185,15 +204,17 @@ func TestACalcOperandIsHeldAgainstEveryRoute(t *testing.T) {
// rejected, so the reference spelling has to be. // rejected, so the reference spelling has to be.
"a reference into the operand": { "a reference into the operand": {
map[string]string{ map[string]string{
"cat": `{"format":"{calc(net * 2, 2)}|{/cat.net.v}",` + "cat": `{"format":"{calc(net * 2, 2)}|{/hop}",` +
`"net":{"format":"{v}","v":["10.00","20.00"]}}`, `"net":{"format":"{v}","v":["10.00","20.00"]}}`,
"hop": `"{/cat.net.v}"`,
}, },
`{/cat.net.v} renders "net"`, `{/hop} renders "net"`,
}, },
"a reference into the operand one level down": { "a reference into the operand one level down": {
map[string]string{ map[string]string{
"cat": `{"format":"{calc(net * 2, 2)}|{q}","net":{"format":"{v}","v":["10.00","20.00"]},` + "cat": `{"format":"{calc(net * 2, 2)}|{q}","net":{"format":"{v}","v":["10.00","20.00"]},` +
`"q":"{/cat.net.v}"}`, `"q":"{/hop}"}`,
"hop": `"{/cat.net.v}"`,
}, },
`{q} renders "net"`, `{q} renders "net"`,
}, },
@@ -203,7 +224,8 @@ func TestACalcOperandIsHeldAgainstEveryRoute(t *testing.T) {
"a violation on the second of two held heads": { "a violation on the second of two held heads": {
map[string]string{ map[string]string{
"cat": `{"format":"{calc(a + b, 0)} {w}","a":{"format":"{x}","x":["1","2"]},` + "cat": `{"format":"{calc(a + b, 0)} {w}","a":{"format":"{x}","x":["1","2"]},` +
`"b":{"format":"{y}","y":["3","4"]},"w":"{/cat.b}"}`, `"b":{"format":"{y}","y":["3","4"]},"w":"{/hop}"}`,
"hop": `"{/cat.b}"`,
}, },
`{w} renders "b"`, `{w} renders "b"`,
}, },
@@ -230,7 +252,8 @@ func TestACalcOperandIsHeldAgainstEveryRoute(t *testing.T) {
"a reference to a sibling the operand never renders": { "a reference to a sibling the operand never renders": {
"cat": `{"format":"{calc(net * 2, 2)} {unit}",` + "cat": `{"format":"{calc(net * 2, 2)} {unit}",` +
`"net":{"format":"{v}","v":["1","2"],"spare":["kg","lb"]},` + `"net":{"format":"{v}","v":["1","2"],"spare":["kg","lb"]},` +
`"unit":"{/cat.net.spare}"}`, `"unit":"{/hop}"}`,
"hop": `"{/cat.net.spare}"`,
}, },
// Two operands drawing from one source are two names, so two draws: each is // Two operands drawing from one source are two names, so two draws: each is
// held under its own name and shown once, and neither can disagree. // held under its own name and shown once, and neither can disagree.
@@ -247,7 +270,8 @@ func TestACalcOperandIsHeldAgainstEveryRoute(t *testing.T) {
}, },
// A fixed string cannot disagree with itself, so it needs no fence. // A fixed string cannot disagree with itself, so it needs no fence.
"a literal operand named twice": { "a literal operand named twice": {
"cat": `{"format":"{calc(n * 2, 0)} {/cat.n}","n":"5"}`, "cat": `{"format":"{calc(n * 2, 0)} {/hop}","n":"5"}`,
"hop": `"{/cat.n}"`,
}, },
// One node reached twice while walking the operand: the walk must not // One node reached twice while walking the operand: the walk must not
// revisit it, and the repeat is not a second route to anything. // revisit it, and the repeat is not a second route to anything.
@@ -267,19 +291,28 @@ func TestAPathReachesEveryVariantItMightDraw(t *testing.T) {
// all of them. Reaching only the first leaves whatever hides in a later // all of them. Reaching only the first leaves whatever hides in a later
// variant to be found at render — a cycle there is fatal, and a second route // variant to be found at render — a cycle there is fatal, and a second route
// to a held level disagrees silently. // to a held level disagrees silently.
rejected := map[string]struct{ file, want string }{ rejected := map[string]struct {
files map[string]string
want string
}{
"a cycle in a later variant": { "a cycle in a later variant": {
`{"format":"{p.x}","p":[{"format":"h","x":"safe"},{"format":"h","x":"{/cat}"}]}`, map[string]string{
"cat": `{"format":"{p.x}","p":[{"format":"h","x":"safe"},{"format":"h","x":"{/hop}"}]}`,
"hop": `"{/cat}"`,
},
"reference cycle", "reference cycle",
}, },
"a second route in a later variant": { "a second route in a later variant": {
`{"format":"{p.x} {q.y}","p":[{"format":"h","x":"safe"},{"format":"h","x":"{/cat.q}"}],` + map[string]string{
"cat": `{"format":"{p.x} {q.y}","p":[{"format":"h","x":"safe"},{"format":"h","x":"{/hop}"}],` +
`"q":{"format":"{y}","y":["1","2"]}}`, `"q":{"format":"{y}","y":["1","2"]}}`,
"hop": `"{/cat.q}"`,
},
"reads a path into", "reads a path into",
}, },
} }
for name, c := range rejected { for name, c := range rejected {
_, err := New(WithoutShippedData(), WithDataPath(writeData(t, map[string]string{"cat": c.file}))) _, err := New(WithoutShippedData(), WithDataPath(writeData(t, c.files)))
if err == nil || !strings.Contains(err.Error(), c.want) { if err == nil || !strings.Contains(err.Error(), c.want) {
t.Errorf("%s: New = %v, want it to mention %q", name, err, c.want) t.Errorf("%s: New = %v, want it to mention %q", name, err, c.want)
} }
@@ -289,15 +322,22 @@ func TestAPathReachesEveryVariantItMightDraw(t *testing.T) {
func TestALevelAPathNeverRendersIsAccepted(t *testing.T) { func TestALevelAPathNeverRendersIsAccepted(t *testing.T) {
// A path token does not expand its head's format, so a reference sitting in // A path token does not expand its head's format, so a reference sitting in
// that format is not a second route to anything: it is never rendered by the // that format is not a second route to anything: it is never rendered by the
// path at all. Both orders must load. // path at all. Both orders must load. p and q sit one level down, so they are
accepted := map[string]string{ // one column of thing rather than two that would read one another.
"reference in the head's own format": `{"format":"{p.first} {q.a}",` + accepted := map[string]map[string]string{
`"p":{"format":"{first} {/thing.q}","first":["A","B"]},"q":{"format":"{a}","a":["1","2"]}}`, "reference in the head's own format": {
"the mirror shape": `{"format":"{p.first} {q.a}",` + "thing": `{"format":"{inner}","inner":{"format":"{p.first} {q.a}",` +
`"p":{"format":"{first}","first":["A","B"]},"q":{"format":"{a} {/thing.p}","a":["1","2"]}}`, `"p":{"format":"{first} {/hop}","first":["A","B"]},"q":{"format":"{a}","a":["1","2"]}}}`,
"hop": `"{/thing.inner.q}"`,
},
"the mirror shape": {
"thing": `{"format":"{inner}","inner":{"format":"{p.first} {q.a}",` +
`"p":{"format":"{first}","first":["A","B"]},"q":{"format":"{a} {/hop}","a":["1","2"]}}}`,
"hop": `"{/thing.inner.p}"`,
},
} }
for name, file := range accepted { for name, files := range accepted {
if _, err := New(WithoutShippedData(), WithDataPath(writeData(t, map[string]string{"thing": file}))); err != nil { if _, err := New(WithoutShippedData(), WithDataPath(writeData(t, files))); err != nil {
t.Errorf("%s: New = %v, want it accepted", name, err) t.Errorf("%s: New = %v, want it accepted", name, err)
} }
} }
@@ -673,7 +713,8 @@ func TestCycleThroughAPathTokenIsRejected(t *testing.T) {
// must be caught at New. Reaching render would be fatal: the recursion never // must be caught at New. Reaching render would be fatal: the recursion never
// terminates, and a stack overflow cannot be recovered. // terminates, and a stack overflow cannot be recovered.
_, err := New(WithoutShippedData(), WithDataPath(writeData(t, map[string]string{ _, err := New(WithoutShippedData(), WithDataPath(writeData(t, map[string]string{
"a": `{"format":"{p.x}","p":{"format":"{x}","x":"{/a}"}}`, "a": `{"format":"{p.x}","p":{"format":"{x}","x":"{/b}"}}`,
"b": `"{/a}"`,
}))) })))
if err == nil || !strings.Contains(err.Error(), "reference cycle") { if err == nil || !strings.Contains(err.Error(), "reference cycle") {
t.Fatalf("New = %v, want the cycle through {p.x} rejected", err) t.Fatalf("New = %v, want the cycle through {p.x} rejected", err)
+1 -1
View File
@@ -164,6 +164,6 @@ func linkNodeRefs(scope nodeScope, root map[string]node) error {
return fmt.Errorf("%s: reference {%s}: an inline template has no folder; write {/%s}", path, name, rest) return fmt.Errorf("%s: reference {%s}: an inline template has no folder; write {/%s}", path, name, rest)
} }
} }
return linkTemplateRefs(nil, path, t, root) return linkTemplateRefs(nil, path, "", t, root)
}) })
} }
-3
View File
@@ -211,9 +211,6 @@ func recordOf(n node) (*template, []Column, error) {
if !t.record { if !t.record {
return nil, nil, fmt.Errorf("carries repeat %d, which composes its format into one string; a record projects columns instead — drop the repeat and render the record again for more rows", t.repeat) return nil, nil, fmt.Errorf("carries repeat %d, which composes its format into one string; a record projects columns instead — drop the repeat and render the record again for more rows", t.repeat)
} }
if err := checkColumnDraws(t, names); err != nil {
return nil, nil, err
}
columns := make([]Column, len(names)) columns := make([]Column, len(names))
for i, name := range names { for i, name := range names {
datatype, _ := columnDatatype(t.fields[name]) // checkColumns refused items that disagree wherever DataType is read datatype, _ := columnDatatype(t.fields[name]) // checkColumns refused items that disagree wherever DataType is read
+5 -9
View File
@@ -217,18 +217,14 @@ func TestRecordRejectsOverlappingReferenceColumns(t *testing.T) {
{"as a builtin operand", `{"format":"","whole":"{uppercase(/cat.a)}","inner":"{/cat.a.b}"}`}, {"as a builtin operand", `{"format":"","whole":"{uppercase(/cat.a)}","inner":"{/cat.a.b}"}`},
{"a bare reference beside a path", `{"format":"","whole":"{/cat}","inner":"{/cat.a.b}"}`}, {"a bare reference beside a path", `{"format":"","whole":"{/cat}","inner":"{/cat.a.b}"}`},
} { } {
f := newGenerator(t, writeData(t, map[string]string{"cat": cat, "row": c.row}), WithSeed(1)) _, err := New(WithoutShippedData(), WithDataPath(writeData(t, map[string]string{"cat": cat, "row": c.row})))
_, err := f.FakeRecord("row")
if err == nil || !strings.Contains(err.Error(), "reads a path into") { if err == nil || !strings.Contains(err.Error(), "reads a path into") {
t.Errorf("%s: FakeRecord = %v, want the overlap rejected the way one format is", c.name, err) t.Errorf("%s: New = %v, want the overlap refused at load, the way one format is", c.name, err)
continue continue
} }
if !strings.Contains(err.Error(), `"whole"`) || !strings.Contains(err.Error(), `"inner"`) { 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) t.Errorf("%s: error %q names neither column; it must name both", c.name, err)
} }
if _, err := f.Fake("row"); err != nil {
t.Errorf("%s: Fake(row) = %v, want the string view untouched", c.name, err)
}
} }
} }
@@ -250,9 +246,9 @@ func TestRecordRejectsAColumnReadingItsOwnRecord(t *testing.T) {
{"the record as an operand", `"up":"{uppercase(/person)}"`}, {"the record as an operand", `"up":"{uppercase(/person)}"`},
} { } {
person := `{"format":"{first} {last}","first":["Ada","Bo"],"last":["Lovelace","Ek"],` + c.column + `}` person := `{"format":"{first} {last}","first":["Ada","Bo"],"last":["Lovelace","Ek"],` + c.column + `}`
f := newGenerator(t, writeData(t, map[string]string{"person": person}), WithSeed(1)) _, err := New(WithoutShippedData(), WithDataPath(writeData(t, map[string]string{"person": person})))
if _, err := f.FakeRecord("person"); err == nil || !strings.Contains(err.Error(), "points back at this record") { if err == nil || !strings.Contains(err.Error(), "names the category it sits in") {
t.Errorf("%s: FakeRecord = %v, want it refused; the column would contradict the columns beside it", c.name, err) t.Errorf("%s: New = %v, want it refused at load; the column would contradict the columns beside it", c.name, err)
} }
} }
} }
+11 -6
View File
@@ -72,15 +72,17 @@ func refSegments(name string, folder []string) ([]string, error) {
// error, never a random render-time one. // error, never a random render-time one.
func linkRefs(root map[string]node) error { func linkRefs(root map[string]node) error {
return eachTemplate(root, func(folder []string, path string, t *template) error { return eachTemplate(root, func(folder []string, path string, t *template) error {
t.keyDrawGroup(strings.Join(strings.Split(path, ".")[:len(folder)+1], ".")) category := strings.Join(strings.Split(path, ".")[:len(folder)+1], ".")
return linkTemplateRefs(folder, path, t, root) t.keyDrawGroup(category)
return linkTemplateRefs(folder, path, category, t, root)
}) })
} }
// linkTemplateRefs binds one template's references against root. A template with // linkTemplateRefs binds one template's references against root, refusing one that names the
// none is left untouched, so an inline format that references nothing costs only // category it sits in: a category is a unit, and a reference back into it describes a draw other
// the refTokens scan. // than the fields beside it. A template with no reference is left untouched, so an inline format
func linkTemplateRefs(folder []string, path string, t *template, root map[string]node) error { // that references nothing costs only the refTokens scan.
func linkTemplateRefs(folder []string, path, category string, t *template, root map[string]node) error {
names := refTokens(t.format) names := refTokens(t.format)
if len(names) == 0 { if len(names) == 0 {
return nil return nil
@@ -99,6 +101,9 @@ func linkTemplateRefs(folder []string, path string, t *template, root map[string
return fmt.Errorf("%s: reference {%s}: %w", path, name, err) return fmt.Errorf("%s: reference {%s}: %w", path, name, err)
} }
key := "/" + strings.Join(head, ".") key := "/" + strings.Join(head, ".")
if category != "" && key == "/"+category {
return fmt.Errorf("%s: reference {%s}: names the category it sits in; read a sibling field as a path, or move the shared value into its own category and reference that", path, name)
}
if err := checkPath(target, tail, key); err != nil { if err := checkPath(target, tail, key); err != nil {
return fmt.Errorf("%s: reference {%s}: %w", path, name, err) return fmt.Errorf("%s: reference {%s}: %w", path, name, err)
} }
+52 -36
View File
@@ -83,25 +83,23 @@ func TestReferenceErrors(t *testing.T) {
"card": `"{/who.f}"`, "card": `"{/who.f}"`,
}, },
"empty reference path": {"card": `"{/}"`}, "empty reference path": {"card": `"{/}"`},
// A reference that leads back to its own value never terminates at render, // A reference that leads back to its own value never terminates at render, so
// so New must reject the cycle up front (direct, mutual, or chained). // New must reject the cycle up front (mutual or chained). One into its own
"direct cycle": {"a": `"x{/a}"`}, // category is refused before the cycle walk reaches it, as a unit rule.
"a category referencing itself": {"a": `"x{/a}"`},
"mutual cycle": {"a": `"{/b}"`, "b": `"{/a}"`}, "mutual cycle": {"a": `"{/b}"`, "b": `"{/a}"`},
"chain cycle": {"a": `"{/b}"`, "b": `"{/c}"`, "c": `"{/a}"`}, "chain cycle": {"a": `"{/b}"`, "b": `"{/c}"`, "c": `"{/a}"`},
// calc renders its operands, so a cycle through one must be caught too. // calc renders its operands, so a reference through one is caught too.
"calc operand cycle": {"x": `{"format":"{calc(y)}","y":"{/x}"}`}, "a calc operand into its own category": {"x": `{"format":"{calc(y)}","y":"{/x}"}`},
// A field its parent's format never renders is still reachable by dot path, // A field its parent's format never renders is still reachable by dot path,
// so a cycle hiding in one must fail at New rather than at render. // so what hides in one must fail at New rather than at render.
"cycle in an unrendered field": {"cat": `{"format":"hi","x":"{/cat.x}"}`}, "an unrendered field into its own category": {"cat": `{"format":"hi","x":"{/cat.x}"}`},
"mutual cycle between unrendered fields": { "two unrendered fields into their own category": {
"cat": `{"format":"hi","x":"{/cat.y}","y":"{/cat.x}"}`, "cat": `{"format":"hi","x":"{/cat.y}","y":"{/cat.x}"}`,
}, },
"cycle in an unrendered field of a choice arm": { // The shipped layout puts categories in folders, so one level down is the
"cat": `{"format":"hi","x":"{/cat.x}"}`, // common case, not an edge case.
}, "a category in a subfolder referencing itself": {"sv_SE/a": `"x{/sv_SE.a}"`},
// The shipped layout puts categories in folders, so a cycle one level down
// is the common case, not an edge case.
"cycle in a subfolder": {"sv_SE/a": `"x{/sv_SE.a}"`},
"mutual cycle within a subfolder": {"sv_SE/a": `"{/sv_SE.b}"`, "sv_SE/b": `"{/sv_SE.a}"`}, "mutual cycle within a subfolder": {"sv_SE/a": `"{/sv_SE.b}"`, "sv_SE/b": `"{/sv_SE.a}"`},
"mutual cycle across two folders": {"en_US/a": `"{/sv_SE.b}"`, "sv_SE/b": `"{/en_US.a}"`}, "mutual cycle across two folders": {"en_US/a": `"{/sv_SE.b}"`, "sv_SE/b": `"{/en_US.a}"`},
// ".." is reserved for bound references, so an authored key using it would // ".." is reserved for bound references, so an authored key using it would
@@ -131,17 +129,26 @@ func TestDotPrefixedDataEntriesAreSkipped(t *testing.T) {
} }
} }
// TestReferenceFromUnrenderedFieldTerminates guards the cycle check against // TestReferenceIntoItsOwnCategoryIsRejected pins the unit a category is: a reference
// over-rejecting: a field the format never renders may point back at its own // back into it describes a draw other than the fields beside it, whether the format
// category, which terminates, and stays renderable by path. // renders that field or not, so it is refused at load rather than left to disagree in
func TestReferenceFromUnrenderedFieldTerminates(t *testing.T) { // the record view.
dir := writeData(t, map[string]string{"cat": `{"format":"hi","x":"see {/cat}"}`}) func TestReferenceIntoItsOwnCategoryIsRejected(t *testing.T) {
f := newGenerator(t, dir, WithSeed(1)) for name, file := range map[string]string{
if got := fake(t, f, "cat"); got != "hi" { "the category whole": `{"format":"hi","x":"see {/cat}"}`,
t.Fatalf("cat = %q, want hi", got) "a field of its own": `{"format":"hi","x":"{/cat.y}","y":"1"}`,
"a path the format reads": `{"format":"{/cat.y}","y":"1"}`,
} {
_, err := New(WithoutShippedData(), WithDataPath(writeData(t, map[string]string{"cat": file})))
if err == nil || !strings.Contains(err.Error(), "names the category it sits in") {
t.Errorf("%s: New = %v, want the reference into its own category refused", name, err)
} }
if got := fake(t, f, "cat.x"); got != "see hi" { }
t.Fatalf("cat.x = %q, want \"see hi\"", got) // Reading a sibling as a path is the spelling that stays.
if _, err := New(WithoutShippedData(), WithDataPath(writeData(t, map[string]string{
"cat": `{"format":"{y.v}","y":{"format":"{v}","v":["1","2"]}}`,
}))); err != nil {
t.Errorf("New = %v, want the sibling path accepted", err)
} }
} }
@@ -189,9 +196,9 @@ func TestNewErrorPathIsCanonical(t *testing.T) {
want string want string
}{ }{
{ {
"cycle inside a choice arm", "cycle through another category",
map[string]string{"cat": `{"format":"hi","x":"{/cat.x}"}`}, map[string]string{"cat": `{"format":"hi","x":"{/hop}"}`, "hop": `"{/cat.x}"`},
"fejkdata: reference cycle: cat.x -> /cat.x", "fejkdata: reference cycle: cat.x -> /hop -> /cat.x",
}, },
{ {
"bad reference reached through another reference", "bad reference reached through another reference",
@@ -255,21 +262,30 @@ func TestBareReferenceDrawsEachTime(t *testing.T) {
} }
func TestReferenceOverlapIsRejected(t *testing.T) { func TestReferenceOverlapIsRejected(t *testing.T) {
p := `[{"format":"{first}","first":"A","last":"1"},{"format":"{first}","first":"B","last":"2"}]` // A category never references itself, so the reads sit in a second category.
for name, file := range map[string]string{ cat := `{"format":"x","p":[{"format":"{first}","first":"A","last":"1"},{"format":"{first}","first":"B","last":"2"}]}`
"head beside a path": `{"format":"{/cat.p} {/cat.p.first}","p":` + p + `}`, for name, row := range map[string]string{
"sibling path beside a reference path": `{"format":"{p.first} {/cat.p.last}","p":` + p + `}`, "head beside a path": `"{/cat.p} {/cat.p.first}"`,
"sibling fields reading a level and a path into it": `{"format":"{a} {b}","a":"{/cat.p}","b":"{/cat.p.first}","p":` + p + `}`, "sibling fields reading a level and a path into it": `{"format":"{a} {b}","a":"{/cat.p}","b":"{/cat.p.first}"}`,
"a field rendering the level a nested reference reads into": `{"format":"{x} {p}","x":"{/cat.p.first}","p":` + p + `}`, "a field rendering the level a nested reference reads into": `{"format":"{x} {p}","x":"{/cat.p.first}","p":"{/cat.p}"}`,
"a bare reference beside a path into what it never renders": `{"format":"{a} {b}","a":"{/other}","b":"{/other.p.first}"}`, "a bare reference beside a path into what it never renders": `{"format":"{a} {b}","a":"{/cat}","b":"{/cat.p.first}"}`,
} { } {
_, err := New(WithoutShippedData(), WithDataPath(writeData(t, map[string]string{"cat": file, "other": `{"format":"x","p":` + p + `}`}))) _, err := New(WithoutShippedData(), WithDataPath(writeData(t, map[string]string{"cat": cat, "row": row})))
if err == nil || !strings.Contains(err.Error(), "reads a path into") { if err == nil || !strings.Contains(err.Error(), "reads a path into") {
t.Errorf("%s: New = %v, want the overlap rejected", name, err) t.Errorf("%s: New = %v, want the overlap rejected", name, err)
} }
} }
// A sibling path and a reference into the level it holds are the same overlap,
// and the reference reaches it from a category row renders.
if _, err := New(WithoutShippedData(), WithDataPath(writeData(t, map[string]string{ if _, err := New(WithoutShippedData(), WithDataPath(writeData(t, map[string]string{
"cat": `{"format":"{a} {b}","a":{"format":"{/cat.p}","drawGroup":"g"},"b":"{/cat.p.first}","p":` + p + `}`, "row": `{"format":"{p.first} {/hop}","p":[{"format":"{first}","first":"A","last":"1"},{"format":"{first}","first":"B","last":"2"}]}`,
"hop": `"{/row.p.last}"`,
}))); err == nil || !strings.Contains(err.Error(), "reads a path into") {
t.Errorf("New = %v, want a sibling path beside a reference into it rejected", err)
}
if _, err := New(WithoutShippedData(), WithDataPath(writeData(t, map[string]string{
"cat": cat,
"row": `{"format":"{a} {b}","a":{"format":"{/cat.p}","drawGroup":"g"},"b":"{/cat.p.first}"}`,
}))); err != nil { }))); err != nil {
t.Errorf("New = %v, want a level and a path into it accepted in groups of their own", err) t.Errorf("New = %v, want a level and a path into it accepted in groups of their own", err)
} }