Answer architecture round 2: refuse by default where a builtin's operand can be a reference, test the flag against each operand grammar, drop a duplicate test, rename allNeverNumeric, rewrap Computation, and order item 91 after item 4
This commit is contained in:
@@ -684,8 +684,8 @@ hyphenated field or name can't be an operand.
|
||||
{ "format": "{net} x {qty} = {calc(net * qty, 2)}", "net": ["19.99", "5.00"], "qty": ["3", "7"] }
|
||||
```
|
||||
|
||||
Renders e.g. `19.99 x 3 = 59.97`. A name is the operand `{lat}` prints, so the calc computes
|
||||
from the pick shown:
|
||||
Renders e.g. `19.99 x 3 = 59.97`. A name is the operand `{lat}` prints, so the calc
|
||||
computes from the pick shown:
|
||||
|
||||
```json
|
||||
"{/misc.coordinate.lat as lat}{lat}° is {calc(lat * 60, 0)} arcminutes"
|
||||
@@ -693,9 +693,9 @@ from the pick shown:
|
||||
|
||||
Renders e.g. `5.338477° is 320 arcminutes`.
|
||||
|
||||
A result that rounds to zero prints unsigned — `0`, `0.00` — as `{float()}`'s does. An operand that can never be a number (`"abc"`,
|
||||
or a choice of such) is rejected at load, as is a division by a constant zero
|
||||
(`1/0`, or a fixed `"0"` field); an operand that sometimes is not a number yields
|
||||
A result that rounds to zero prints unsigned — `0`, `0.00` — as `{float()}`'s does. An
|
||||
operand that can never be a number (`"abc"`, or a choice of such) is rejected at load, as
|
||||
is a division by a constant zero (`1/0`, or a fixed `"0"` field); an operand that sometimes is not a number yields
|
||||
`NaN`, and a division by one that is not constant `Inf` — both print rather than
|
||||
fail, except in a [typed column](#datatype), which must prove neither happens.
|
||||
|
||||
|
||||
+1
-1
@@ -64,7 +64,7 @@ var builtins = withTransforms(map[string]builtin{
|
||||
}},
|
||||
"date": {arity: -1, checkArgs: dateArgs, prep: datePrep},
|
||||
"time": {arity: -1, checkArgs: timeArg, prep: timePrep},
|
||||
"calc": {arity: -1, checkArgs: checkCalc, prep: calcPrep, operands: calcOperands},
|
||||
"calc": {arity: -1, checkArgs: checkCalc, prep: calcPrep, operands: calcOperands, noRefOperands: true},
|
||||
// seq is the one stateful builtin: a per-generator counter from 1, advancing on
|
||||
// each call. An optional name selects an independent counter; no name uses the
|
||||
// default one. Deterministic by construction, so seeded output stays stable.
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"encoding/base64"
|
||||
"fmt"
|
||||
"regexp"
|
||||
"slices"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
@@ -447,3 +448,14 @@ func TestArgErrorsNameTheSpelling(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestNoRefOperandsMatchesTheOperandGrammar(t *testing.T) {
|
||||
for name, b := range builtins {
|
||||
if b.operands == nil {
|
||||
continue
|
||||
}
|
||||
if takesRef := slices.ContainsFunc(b.operands([]string{"/x"}), isRef); b.noRefOperands == takesRef {
|
||||
t.Errorf("%s: noRefOperands = %v, but its operands func reads a reference: %v", name, b.noRefOperands, takesRef)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -98,7 +98,7 @@ func checkCalcNames(path string, t *template) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// operandNodes lists every node each operand of o may render, once linkNames compiled the names.
|
||||
// operandNodes lists every node each operand of o may render, once its template and names link.
|
||||
func operandNodes(o op) func(name string) []node {
|
||||
return func(name string) []node {
|
||||
a := o.operands[slices.IndexFunc(o.operands, func(a arm) bool { return a.head == name })]
|
||||
@@ -113,7 +113,7 @@ func operandNodes(o op) func(name string) []node {
|
||||
// operand lists every node an operand may render, nil while that is unknown.
|
||||
func checkOperands(text string, expr calcNode, operand func(name string) []node) error {
|
||||
for _, name := range calcVars(expr) {
|
||||
if rendered, never := noneNumeric(operand(name)); never {
|
||||
if rendered, never := allNeverNumeric(operand(name)); never {
|
||||
return fmt.Errorf("calc(%q): operand %q is never a number: it renders %q", text, name, rendered)
|
||||
}
|
||||
}
|
||||
@@ -207,13 +207,13 @@ func neverNumeric(n node) (text string, never bool) {
|
||||
return lit, true
|
||||
}
|
||||
case *choice:
|
||||
return noneNumeric(n.items)
|
||||
return allNeverNumeric(n.items)
|
||||
}
|
||||
return "", false
|
||||
}
|
||||
|
||||
// noneNumeric reports nodes no render of which is a number, and none where there are none.
|
||||
func noneNumeric(nodes []node) (text string, never bool) {
|
||||
// allNeverNumeric reports nodes no render of which is a number, and none where there are none.
|
||||
func allNeverNumeric(nodes []node) (text string, never bool) {
|
||||
for _, n := range nodes {
|
||||
t, nodeNever := neverNumeric(n)
|
||||
if !nodeNever {
|
||||
|
||||
@@ -133,9 +133,9 @@ type builtin struct {
|
||||
// operands names the fields the call reads, which expand renders for it; nil
|
||||
// for a builtin that reads none.
|
||||
operands func(args []string) []string
|
||||
// refOperands says an operand may be a reference, so a name read once there has the bare
|
||||
// reference as a second spelling.
|
||||
refOperands bool
|
||||
// noRefOperands says no operand can be a reference, so a name read once there has no
|
||||
// second spelling.
|
||||
noRefOperands bool
|
||||
// proveNumber bounds the number a call's text reads as, token its body, and says which
|
||||
// datatypes that text is not; set it where every render reads as a finite number, which
|
||||
// makes the call a calc operand, and leave it nil otherwise.
|
||||
@@ -281,7 +281,7 @@ func checkReads(t formatToken, fields map[string]node, operands bool) ([]unbound
|
||||
return nil, err
|
||||
}
|
||||
a := splitArm(name, nil)
|
||||
unbound = append(unbound, unboundRead{head: a.head, tail: joinSegments(a.tail), body: t.body, operand: operands, noRef: operands && !builtins[t.fn].refOperands, err: err})
|
||||
unbound = append(unbound, unboundRead{head: a.head, tail: joinSegments(a.tail), body: t.body, operand: operands, noRef: operands && builtins[t.fn].noRefOperands, err: err})
|
||||
}
|
||||
return unbound, nil
|
||||
}
|
||||
|
||||
@@ -206,8 +206,6 @@ func TestNameErrors(t *testing.T) {
|
||||
`write {/word}`},
|
||||
{"read once whole, as a transform's operand", map[string]string{"word": `["a","b"]`, "card": `"{/word as w}{uppercase(w)}"`},
|
||||
`write /word where it is read`},
|
||||
{"read once whole, as a calc's operand", map[string]string{"n": `["2","3"]`, "card": `"{/n as k}{calc(k * 2)}"`},
|
||||
""},
|
||||
{"rendered twice where a path reads it", map[string]string{"word": `{"format":"{w}-{w}","w":["a","b"]}`, "card": `"{/word as n}{n}|{n.w}"`},
|
||||
`{n} renders field "w" twice`},
|
||||
{"a binding in a cell", map[string]string{"word": `["a","b"]`, "t.json": `{"format":"{k}","rows":"t.tsv","key":"k"}`, "t.tsv": "k\tc\nx\t{/word as w}{w}{w}\ny\tz\n"},
|
||||
|
||||
@@ -231,7 +231,7 @@ Shape: T = table, t = template, c = choice.
|
||||
- Deletes the token-order rules (`drawsApart`, `readFold`, `branches`, the pairwise replay), the expansion hold fence, `drawGroup`, the own-family fence and the cell-reference fences, and revises the decisions that point here. The remaining draw checks are names, paths through names, cycles and the repeat cap.
|
||||
- Fixes on the way: `{/sel}|{/sel}` panics out of `Fake` today where two draws land on rows whose cells select different rows of another table; it must render.
|
||||
- Rewrites the README's Computation, References, Draw group, Correlated fields, Linked tables, Names and "One draw, one spelling" sections; the Linked tables section says which pick's weights govern a family, since the bound row decides.
|
||||
- Keeps `{/n as k}{calc(k * 2)}` loading, since a calc operand cannot be a reference. Widens the refusal of a name read once to every binding, a path into it or a reference path bound included, since `{n.x}` read once is then `{/ref.x}`, and deletes the names fence over draw groups and nested repeats (`checkUses`, `readsHeld`).
|
||||
- Keeps a name read once where no reference can stand, such as `{/n as k}{calc(k * 2)}`, loading. Widens the refusal of a name read once to every binding, a path into it or a reference path bound included, since `{n.x}` read once is then `{/ref.x}`, and deletes the names fence over draw groups and nested repeats (`checkUses`, `readsHeld`).
|
||||
- Say how a record's columns and a struct's tags read one pick, so the `first` and `sex` columns of one `sv_SE.person` record, or one struct's tags reading them, still describe one person.
|
||||
- Re-pin seeded output and `testdata/shipped_shape.txt` in their own commits; the CHANGELOG names the grammar change.
|
||||
|
||||
@@ -249,7 +249,7 @@ Needs items 4 and 90. The decision "comprehension floor: every dimension and the
|
||||
|
||||
### 91. Accept a path through a name as a `calc` operand, so `{calc(t.population * 2)}` computes from the row `{t}` prints.
|
||||
|
||||
A calc operand parses as a bare identifier, so `{/misc.territory as t}{calc(t.population * 2)}` fails with `unexpected ".population"`, and no binding can hold the column alone beside the row. Item 4 removes today's route, a field holding the reference path, which the expansion hold keeps for the render. Resolve the path through `linkName`'s leaves, as `operandNodes` does for a bare name.
|
||||
Needs item 4. A calc operand parses as a bare identifier, so `{/misc.territory as t}{calc(t.population * 2)}` fails with `unexpected ".population"`, and no binding can hold the column alone beside the row. Item 4 removes today's route, a field holding the reference path, which the expansion hold keeps for the render. Resolve the path through `linkName`'s leaves, as `operandNodes` does for a bare name.
|
||||
|
||||
### 89. Fail `New` without `WithSeed`, naming the option, and seed the CLI from the system.
|
||||
|
||||
|
||||
+1
-1
@@ -19,7 +19,7 @@ func withTransforms(calls map[string]builtin) map[string]builtin {
|
||||
if _, clash := calls[name]; clash {
|
||||
panic(internalError("%s is registered as a builtin and a transform", name))
|
||||
}
|
||||
calls[name] = builtin{arity: 1, checkArgs: transformArg, prep: transformPrep(fn), operands: transformOperand, refOperands: true}
|
||||
calls[name] = builtin{arity: 1, checkArgs: transformArg, prep: transformPrep(fn), operands: transformOperand}
|
||||
}
|
||||
return calls
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user