From f9647ff6bbb1d615b8679606c3bc23e06abe67b9 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Wed, 2 Sep 2026 18:31:38 +0200 Subject: [PATCH] Decompose compileTemplate, checkBoundLevelsHeld and loadDir; gate cyclomatic complexity at 14 --- Dockerfile | 1 + README.md | 3 +- calc.go | 1 + compose.yaml | 4 ++ data.go | 77 +++++++++++++++++++++--------------- hold.go | 107 +++++++++++++++++++++++++++++---------------------- node.go | 74 ++++++++++++++++++++++++----------- template.go | 3 +- 8 files changed, 169 insertions(+), 101 deletions(-) diff --git a/Dockerfile b/Dockerfile index 227d6da..8f04a4a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -13,6 +13,7 @@ RUN go mod download COPY . . RUN go vet ./... && \ + go run github.com/fzipp/gocyclo/cmd/gocyclo@v0.6.0 -over 14 -ignore _test . && \ { unformatted="$(gofmt -l .)"; test -z "$unformatted" || \ { echo "unformatted files:"; echo "$unformatted"; exit 1; }; } && \ go test -race ./... diff --git a/README.md b/README.md index 773d875..e775105 100644 --- a/README.md +++ b/README.md @@ -379,6 +379,7 @@ docker compose run --rm cover # tests with coverage docker compose run --rm bench # benchmarks docker compose run --rm build # compile the library docker compose run --rm vet # go vet +docker compose run --rm cyclo # cyclomatic complexity over 14 (test files excluded) docker compose run --rm dev # interactive shell ``` @@ -391,7 +392,7 @@ docker compose run --rm --user "$(id -u):$(id -g)" tidy # go mod tidy Every pull request runs `docker build .` against both the latest and the lowest supported Go, and must pass before it can be merged. That build is the whole -gate — vet, format check and tests — so run it locally before pushing: +gate — vet, complexity, format check and tests — so run it locally before pushing: ```sh docker build . # latest diff --git a/calc.go b/calc.go index a9a8653..140b84d 100644 --- a/calc.go +++ b/calc.go @@ -242,6 +242,7 @@ func (p *calcParser) binary(next func() (calcNode, error), ops ...byte) (calcNod } } +// factor is a table-shaped scanner, one case per token kind, kept whole on purpose. func (p *calcParser) factor() (calcNode, error) { p.space() if p.pos >= len(p.rs) { diff --git a/compose.yaml b/compose.yaml index fda93dd..bb47873 100644 --- a/compose.yaml +++ b/compose.yaml @@ -42,6 +42,10 @@ services: <<: *go command: go vet ./... + cyclo: + <<: *go + command: go run github.com/fzipp/gocyclo/cmd/gocyclo@v0.6.0 -over 14 -ignore _test . + build: <<: *go command: go build ./... diff --git a/data.go b/data.go index fe78367..3d59bb6 100644 --- a/data.go +++ b/data.go @@ -87,44 +87,59 @@ func loadDir(src dataSource, dir string) (*group, error) { continue } full := path.Join(dir, e.Name()) + load := loadFile if e.IsDir() { - child, err := loadDir(src, full) - if err != nil { - return nil, err - } - if len(child.children) == 0 { - continue - } - if err := checkName(e.Name()); err != nil { - return nil, fmt.Errorf("%s: folder %w", src.name(full), err) - } - g.children[e.Name()] = child - continue + load = loadFolder } - if !strings.HasSuffix(e.Name(), ".json") { - continue + if err := load(src, g, full, e.Name()); err != nil { + return nil, err } - name := strings.TrimSuffix(e.Name(), ".json") - if err := checkName(name); err != nil { - return nil, fmt.Errorf("%s: category %w", src.name(full), err) - } - b, err := fs.ReadFile(src.fsys, full) - if err != nil { - return nil, fmt.Errorf("%s: %w", src.name(full), err) - } - var raw any - if err := json.Unmarshal(b, &raw); err != nil { - return nil, fmt.Errorf("%s: %w", src.name(full), err) - } - n, err := compile(raw) - if err != nil { - return nil, fmt.Errorf("%s: %w", src.name(full), err) - } - g.children[name] = n } return g, nil } +// loadFolder adds a subdirectory as a nested group, unless nothing under it is data. +func loadFolder(src dataSource, g *group, full, name string) error { + child, err := loadDir(src, full) + if err != nil { + return err + } + if len(child.children) == 0 { + return nil + } + if err := checkName(name); err != nil { + return fmt.Errorf("%s: folder %w", src.name(full), err) + } + g.children[name] = child + return nil +} + +// loadFile compiles a *.json file into a category named after it; any other file +// is skipped. +func loadFile(src dataSource, g *group, full, file string) error { + if !strings.HasSuffix(file, ".json") { + return nil + } + name := strings.TrimSuffix(file, ".json") + if err := checkName(name); err != nil { + return fmt.Errorf("%s: category %w", src.name(full), err) + } + b, err := fs.ReadFile(src.fsys, full) + if err != nil { + return fmt.Errorf("%s: %w", src.name(full), err) + } + var raw any + if err := json.Unmarshal(b, &raw); err != nil { + return fmt.Errorf("%s: %w", src.name(full), err) + } + n, err := compile(raw) + if err != nil { + return fmt.Errorf("%s: %w", src.name(full), err) + } + g.children[name] = n + return nil +} + // mergeChildren overlays src onto dst. Two groups under the same key merge // recursively (so locales/categories from several paths combine); every other // key is replaced, making the last-loaded directory win on a conflict. diff --git a/hold.go b/hold.go index 965dce2..def4091 100644 --- a/hold.go +++ b/hold.go @@ -19,59 +19,74 @@ func checkBoundLevelsHeld(root map[string]node) error { if !ok || len(t.held) == 0 { return nil } - heads := make([]string, 0, len(t.held)) - for head := range t.held { - heads = append(heads, head) - } - // Operand heads first, then paths, each in name order: a level read both - // ways is reported by the operand's fence, and which overlap is reported - // does not vary. - sort.Slice(heads, func(i, j int) bool { - _, pi := t.bound[heads[i]] - _, pj := t.bound[heads[j]] - if pi != pj { - return !pi - } - return heads[i] < heads[j] - }) readers := boundReaders(t.format, t.bound, t.refs) - for _, head := range heads { - // What one draw answers for depends on how the draw is read: a path pins - // the levels it passes through and the leaf it lands on, an operand - // exactly the value its render produces. - held := map[node]bool{} - reader, isPath := t.bound[head] - if isPath { - for _, r := range readers { - if a := splitArm(r.name, t.refs); a.key == head { - coverPath(t.fields[head], a.tail, held) - } - } - } else { - operandDraw(t.fields[head], held) - } - if len(held) == 0 { - continue // an early out: a fixed head holds nothing to reach - } - // One seen set across the edges: a node that cannot reach the level - // cannot reach it by another route either, so it is walked once here. - seen := map[node]bool{} - for _, e := range renderEdges(t) { - if splitArm(e.label, t.refs).key == head { - continue // a token or operand reading this draw, the routes allowed - } - if renders(e.to, held, seen) { - if isPath { - return fmt.Errorf("%s: %s renders %q, which {%s} reads a path into; name the fields you want instead", path, e.reached(), head, reader) - } - return fmt.Errorf("%s: %s renders %q, which a {%s()} also reads; reach it one way so it is drawn once", path, e.reached(), head, operandReader(t, head)) - } + for _, head := range heldHeads(t) { + if err := checkHeadHeld(t, head, readers); err != nil { + return fmt.Errorf("%s: %w", path, err) } } return nil }) } +// heldHeads lists a template's held names, operand heads first, then paths, each +// in name order: a level read both ways is reported by the operand's fence, and +// which overlap is reported does not vary. +func heldHeads(t *template) []string { + heads := make([]string, 0, len(t.held)) + for head := range t.held { + heads = append(heads, head) + } + sort.Slice(heads, func(i, j int) bool { + _, pi := t.bound[heads[i]] + _, pj := t.bound[heads[j]] + if pi != pj { + return !pi + } + return heads[i] < heads[j] + }) + return heads +} + +// pinned collects what the hold of head answers for: a path pins the levels it +// passes through and the leaf it lands on, an operand exactly the value its render +// produces. +func pinned(t *template, head string, readers []reader) map[node]bool { + held := map[node]bool{} + if _, isPath := t.bound[head]; !isPath { + operandDraw(t.fields[head], held) + return held + } + for _, r := range readers { + if a := splitArm(r.name, t.refs); a.key == head { + coverPath(t.fields[head], a.tail, held) + } + } + return held +} + +// checkHeadHeld rejects every route to what head's hold pins except the readers +// holding it. One seen set across the edges: a node that cannot reach the level +// cannot reach it by another route either, so it is walked once. +func checkHeadHeld(t *template, head string, readers []reader) error { + held := pinned(t, head, readers) + if len(held) == 0 { + return nil // a fixed head holds nothing to reach + } + reader, isPath := t.bound[head] + seen := map[node]bool{} + for _, e := range renderEdges(t) { + if splitArm(e.label, t.refs).key == head || !renders(e.to, held, seen) { + continue + } + if isPath { + return fmt.Errorf("%s renders %q, which {%s} reads a path into; name the fields you want instead", e.reached(), head, reader) + } + return fmt.Errorf("%s renders %q, which a {%s()} also reads; reach it one way so it is drawn once", e.reached(), head, operandReader(t, head)) + } + return nil +} + // operandReader names the builtin whose operand holds head. func operandReader(t *template, head string) string { fn := "" diff --git a/node.go b/node.go index 6cea876..54b44a3 100644 --- a/node.go +++ b/node.go @@ -181,29 +181,68 @@ func checkNoRepeatedItem(items []any) error { } func compileTemplate(m map[string]any) (node, error) { - format, ok := m["format"].(string) - if !ok { - return nil, fmt.Errorf("template object missing string \"format\"") - } - repeat, err := repeatOf(m) + o, err := readOptions(m) if err != nil { return nil, err } - sep := "" + fields, err := compileFields(m) + if err != nil { + return nil, err + } + if len(fields) == 0 && o.repeat == 1 && !o.weighted { + return nil, fmt.Errorf("an object holding only a format is a string; write %q", o.format) + } + if err := checkTokens(o.format, fields); err != nil { + return nil, err + } + t := &template{format: o.format, fields: fields, repeat: o.repeat, separator: o.separator} + if err := t.compileFormat(); err != nil { + return nil, err + } + return t, nil +} + +// templateOptions is what a template object's option keys say. +type templateOptions struct { + format string + repeat int + separator string + weighted bool +} + +func readOptions(m map[string]any) (templateOptions, error) { + var o templateOptions + format, ok := m["format"].(string) + if !ok { + return o, fmt.Errorf("template object missing string \"format\"") + } + o.format = format + repeat, err := repeatOf(m) + if err != nil { + return o, err + } + o.repeat = repeat if sv, ok := m["separator"]; ok { - if sep, ok = sv.(string); !ok { - return nil, fmt.Errorf("separator must be a string, got %T", sv) + if o.separator, ok = sv.(string); !ok { + return o, fmt.Errorf("separator must be a string, got %T", sv) } if repeat == 1 { - return nil, fmt.Errorf("separator joins repeated renders, so it has no effect without a repeat above 1") + return o, fmt.Errorf("separator joins repeated renders, so it has no effect without a repeat above 1") } } - t := &template{format: format, fields: make(map[string]node, len(m)), repeat: repeat, separator: sep} + _, o.weighted = m["weight"] + return o, nil +} + +// compileFields compiles every non-option key of a template object, in name order +// so which of several bad fields is reported does not vary. +func compileFields(m map[string]any) (map[string]node, error) { + fields := make(map[string]node, len(m)) keys := make([]string, 0, len(m)) for k := range m { keys = append(keys, k) } - sort.Strings(keys) // so which of several bad fields is reported does not vary + sort.Strings(keys) for _, k := range keys { if isOption(k) { continue @@ -215,18 +254,9 @@ func compileTemplate(m map[string]any) (node, error) { if err != nil { return nil, fmt.Errorf("field %q: %w", k, err) } - t.fields[k] = n + fields[k] = n } - if _, weighted := m["weight"]; len(t.fields) == 0 && repeat == 1 && !weighted { - return nil, fmt.Errorf("an object holding only a format is a string; write %q", format) - } - if err := checkTokens(format, t.fields); err != nil { - return nil, err - } - if err := t.compileFormat(); err != nil { - return nil, err - } - return t, nil + return fields, nil } // repeatOf reads a template's "repeat" (default 1): how many times its format diff --git a/template.go b/template.go index 03c057e..87f2ff9 100644 --- a/template.go +++ b/template.go @@ -15,7 +15,8 @@ type ftoken struct { // eachToken scans a format string once and calls fn for each unit, the single // source of truth for how braces are read: "{{" and "}}" are literal braces, a "{" -// opens a token that must reach its "}", and a lone "}" is an error. +// opens a token that must reach its "}", and a lone "}" is an error. A table-shaped +// scanner, one case per rune kind, kept whole on purpose. func eachToken(format string, fn func(ftoken) error) error { rs := []rune(format) for i := 0; i < len(rs); i++ {