Say why an empty name is rejected, and finish what that removed
Tests / vet + fmt + tests (push) Failing after 42s
Tests / vet + fmt + tests (push) Failing after 42s
The reason given was wrong. A dot path did reach an empty-named field:
Fake("a.") returned it, Fake("a.b.") returned a nested one, and {..a.} bound
it. What was true is narrower — List never offered it, because an empty name
is no path segment — so the engine accepted spellings it would not advertise.
The message, the test comment and the README say that instead.
Two things the rejection finished off:
A {} token still reported "no field \"\"", pointing at a fix the loader now
rejects — two errors for one rule. It is told the name can never exist, like
an option token already is.
addressable is dead: both halves of "not empty and no dot" are now rejected
where a name is authored. Mutating it to panic leaves the suite green here
and panics on main, so it was live and is not. Keeping it would preserve the
silent-hiding this change removes — List quietly omitting a name rather than
New refusing it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -238,8 +238,9 @@ option stays unset. `New` rejects an option that cannot take effect — a
|
|||||||
`separator` without a `repeat` above 1, a `weight` outside a choice — and a name
|
`separator` without a `repeat` above 1, a `weight` outside a choice — and a name
|
||||||
using a character the grammars reserve: `.` separates the segments of a path, `|`
|
using a character the grammars reserve: `.` separates the segments of a path, `|`
|
||||||
the arms of a token, `(` opens a function call and `}` ends the token, so a name
|
the arms of a token, `(` opens a function call and `}` ends the token, so a name
|
||||||
carrying one is a name no format could ever spell. That holds for a category, a
|
carrying one is a name no format could ever spell. An empty name goes the same
|
||||||
folder and a field alike.
|
way — it is no path segment at all, so `List` never offers it. That holds for a
|
||||||
|
category, a folder and a field alike.
|
||||||
|
|
||||||
**Functions.** A `{name()}` token calls a built-in function instead of rendering
|
**Functions.** A `{name()}` token calls a built-in function instead of rendering
|
||||||
a field. `{luhn()}` appends a Luhn check digit over the digits emitted **so far**
|
a field. `{luhn()}` appends a Luhn check digit over the digits emitted **so far**
|
||||||
|
|||||||
+10
-4
@@ -120,9 +120,9 @@ func TestNewErrors(t *testing.T) {
|
|||||||
map[string]string{"a|b": `["1"]`},
|
map[string]string{"a|b": `["1"]`},
|
||||||
`category "a|b" contains "|"`,
|
`category "a|b" contains "|"`,
|
||||||
},
|
},
|
||||||
// An empty name is reachable by no dot path, so List cannot advertise it and
|
// An empty name is not a path segment, so List never offered it — while a
|
||||||
// Fake cannot ask for it — {} is the one spelling that reaches it, which is
|
// bare {}, a trailing dot in Fake("a.") and a {..a.} reference all reached
|
||||||
// the same trap {a.} is already rejected for.
|
// it. The engine accepted spellings it would never advertise.
|
||||||
"empty field name": {
|
"empty field name": {
|
||||||
map[string]string{"a": `{"format":"[{}]","":"VALUE"}`},
|
map[string]string{"a": `{"format":"[{}]","":"VALUE"}`},
|
||||||
`field "" is empty`,
|
`field "" is empty`,
|
||||||
@@ -156,9 +156,15 @@ func TestNewErrors(t *testing.T) {
|
|||||||
map[string]string{"a": `{"format":"{..|..}"}`},
|
map[string]string{"a": `{"format":"{..|..}"}`},
|
||||||
"reference has no path",
|
"reference has no path",
|
||||||
},
|
},
|
||||||
|
// No field can be named "", so the token is told that rather than sent to
|
||||||
|
// name one — the fix "no field" points at is itself a load error.
|
||||||
"repeated empty arm": {
|
"repeated empty arm": {
|
||||||
map[string]string{"a": `{"format":"{|}"}`},
|
map[string]string{"a": `{"format":"{|}"}`},
|
||||||
`no field ""`,
|
"a name is never empty",
|
||||||
|
},
|
||||||
|
"bare empty token": {
|
||||||
|
map[string]string{"a": `{"format":"[{}]","x":["1"]}`},
|
||||||
|
"a name is never empty",
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
for name, c := range rejected {
|
for name, c := range rejected {
|
||||||
|
|||||||
@@ -22,7 +22,6 @@ import (
|
|||||||
"fmt"
|
"fmt"
|
||||||
"math/rand/v2"
|
"math/rand/v2"
|
||||||
"sort"
|
"sort"
|
||||||
"strings"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
// Fakes generates fake data from a loaded namespace tree. Create one with [New].
|
// Fakes generates fake data from a loaded namespace tree. Create one with [New].
|
||||||
@@ -93,14 +92,6 @@ func (f *Fakes) List() []string {
|
|||||||
return out
|
return out
|
||||||
}
|
}
|
||||||
|
|
||||||
// addressable reports whether a name can be one segment of a dot path. A dot would
|
|
||||||
// split it into two segments and an empty name into none, so a path through such a
|
|
||||||
// name cannot be spelled — List must not offer one, and it must not count towards
|
|
||||||
// what a choice's variants share.
|
|
||||||
func addressable(name string) bool {
|
|
||||||
return name != "" && !strings.Contains(name, ".")
|
|
||||||
}
|
|
||||||
|
|
||||||
// paths lists the dot paths addressable from n, relative to it, where "" is n
|
// paths lists the dot paths addressable from n, relative to it, where "" is n
|
||||||
// itself. A group has no value of its own, so it contributes only its children's.
|
// itself. A group has no value of its own, so it contributes only its children's.
|
||||||
func paths(n node) []string {
|
func paths(n node) []string {
|
||||||
@@ -108,9 +99,6 @@ func paths(n node) []string {
|
|||||||
case *group:
|
case *group:
|
||||||
var out []string
|
var out []string
|
||||||
for _, name := range sortedNames(n.children) {
|
for _, name := range sortedNames(n.children) {
|
||||||
if !addressable(name) {
|
|
||||||
continue
|
|
||||||
}
|
|
||||||
for _, p := range paths(n.children[name]) {
|
for _, p := range paths(n.children[name]) {
|
||||||
out = append(out, join(name, p))
|
out = append(out, join(name, p))
|
||||||
}
|
}
|
||||||
@@ -119,7 +107,7 @@ func paths(n node) []string {
|
|||||||
case *template:
|
case *template:
|
||||||
out := []string{""}
|
out := []string{""}
|
||||||
for _, name := range sortedNames(n.fields) {
|
for _, name := range sortedNames(n.fields) {
|
||||||
if isRef(name) || !addressable(name) { // a binding, or unreachable by path
|
if isRef(name) { // a binding, not a path segment
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
for _, p := range paths(n.fields[name]) {
|
for _, p := range paths(n.fields[name]) {
|
||||||
|
|||||||
@@ -267,7 +267,7 @@ var reservedList = strings.Join(strings.Split(reservedInName, ""), " ")
|
|||||||
// name may contain.
|
// name may contain.
|
||||||
func checkName(name string) error {
|
func checkName(name string) error {
|
||||||
if name == "" {
|
if name == "" {
|
||||||
return fmt.Errorf("%q is empty, so no dot path can reach it", name)
|
return fmt.Errorf("%q is empty, which is not a path segment, so List never offers it", name)
|
||||||
}
|
}
|
||||||
if i := strings.IndexAny(name, reservedInName); i >= 0 {
|
if i := strings.IndexAny(name, reservedInName); i >= 0 {
|
||||||
return fmt.Errorf("%q contains %q; a name may not use %s, which the dot path and {token} grammars reserve",
|
return fmt.Errorf("%q contains %q; a name may not use %s, which the dot path and {token} grammars reserve",
|
||||||
|
|||||||
@@ -147,6 +147,9 @@ func checkTokens(format string, fields map[string]node) error {
|
|||||||
}
|
}
|
||||||
head, ok := fields[a.key]
|
head, ok := fields[a.key]
|
||||||
if !ok {
|
if !ok {
|
||||||
|
if a.key == "" {
|
||||||
|
return fmt.Errorf("token {%s}: a name is never empty, so this token can name no field", t.body)
|
||||||
|
}
|
||||||
if isOption(a.key) {
|
if isOption(a.key) {
|
||||||
return fmt.Errorf("token {%s}: %q is an option and can never be a field", t.body, a.key)
|
return fmt.Errorf("token {%s}: %q is an option and can never be a field", t.body, a.key)
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user