From e69564dff270f29aa91cd010290b32ef805b7037 Mon Sep 17 00:00:00 2001 From: vickydotbat Date: Wed, 17 Jun 2026 09:52:45 +0200 Subject: [PATCH] Crucible: Fix Formatting Churn --- internal/dispatch/dispatch.go | 6 +- internal/topdata/damagetypes_registry.go | 16 ++- internal/topdata/editorconfig_format.go | 141 +++++++++++++++++++-- internal/topdata/editorconfig_glob_test.go | 56 ++++++++ internal/topdata/itemprops_registry.go | 4 +- internal/topdata/native.go | 4 +- internal/topdata/racialtypes_registry.go | 4 +- internal/topdata/registry_readonly_test.go | 64 ++++++++++ internal/topdata/top_package.go | 2 +- internal/topdata/topdata.go | 4 +- internal/topdata/wiki_native.go | 2 +- 11 files changed, 277 insertions(+), 26 deletions(-) create mode 100644 internal/topdata/editorconfig_glob_test.go create mode 100644 internal/topdata/registry_readonly_test.go diff --git a/internal/dispatch/dispatch.go b/internal/dispatch/dispatch.go index 2f53572..0d5a0b7 100644 --- a/internal/dispatch/dispatch.go +++ b/internal/dispatch/dispatch.go @@ -249,8 +249,10 @@ func usage(w io.Writer) { fmt.Fprintf(w, " list list builders (one per line: namebinsummary)\n") fmt.Fprintf(w, " config [args] inspect/validate effective configuration\n") fmt.Fprintf(w, " changelog [args] generate the release changelog\n") - fmt.Fprintf(w, "\nNWN_ROOT must be passed explicitly (env or flag); crucible never guesses\n") - fmt.Fprintf(w, "from $HOME. See docs/consumer-contract.md.\n") + fmt.Fprintf(w, "\nBuilders read all inputs from the project tree (nwn-tool.yaml + data/); no\n") + fmt.Fprintf(w, "NWN install is required. If a builder ever needs an NWN root, it must be\n") + fmt.Fprintf(w, "passed explicitly via NWN_ROOT; crucible never guesses from $HOME. See\n") + fmt.Fprintf(w, "docs/consumer-contract.md.\n") } func list(w io.Writer) { diff --git a/internal/topdata/damagetypes_registry.go b/internal/topdata/damagetypes_registry.go index 69b9be3..301ba70 100644 --- a/internal/topdata/damagetypes_registry.go +++ b/internal/topdata/damagetypes_registry.go @@ -21,16 +21,20 @@ type damagetypesRegistry struct { Types []map[string]any } -func collectGeneratedRegistryDatasets(dataDir string) ([]nativeCollectedDataset, error) { - itempropsDatasets, err := collectItempropsRegistryDatasets(dataDir) +// collectGeneratedRegistryDatasets projects the registry datasets. When +// persistLocks is true (the build pipeline) newly allocated ids are written +// back to the source lockfiles; when false (validation, discovery, queries) +// collection is read-only and never touches the source tree. +func collectGeneratedRegistryDatasets(dataDir string, persistLocks bool) ([]nativeCollectedDataset, error) { + itempropsDatasets, err := collectItempropsRegistryDatasets(dataDir, persistLocks) if err != nil { return nil, err } - damagetypeDatasets, err := collectDamagetypesRegistryDatasets(dataDir) + damagetypeDatasets, err := collectDamagetypesRegistryDatasets(dataDir, persistLocks) if err != nil { return nil, err } - racialtypesDatasets, err := collectRacialtypesRegistryDatasets(dataDir) + racialtypesDatasets, err := collectRacialtypesRegistryDatasets(dataDir, persistLocks) if err != nil { return nil, err } @@ -70,7 +74,7 @@ func loadDamagetypesRegistry(dataDir string) (*damagetypesRegistry, error) { }, nil } -func collectDamagetypesRegistryDatasets(dataDir string) ([]nativeCollectedDataset, error) { +func collectDamagetypesRegistryDatasets(dataDir string, persistLocks bool) ([]nativeCollectedDataset, error) { registry, err := loadDamagetypesRegistry(dataDir) if err != nil { return nil, err @@ -83,7 +87,7 @@ func collectDamagetypesRegistryDatasets(dataDir string) ([]nativeCollectedDatase if err != nil { return nil, err } - if lockModified { + if lockModified && persistLocks { if err := saveLockfile(registry.LockPath, registry.LockData); err != nil { return nil, err } diff --git a/internal/topdata/editorconfig_format.go b/internal/topdata/editorconfig_format.go index 530e030..03ff8ab 100644 --- a/internal/topdata/editorconfig_format.go +++ b/internal/topdata/editorconfig_format.go @@ -203,8 +203,23 @@ func editorConfigPatternMatches(pattern, relativePath string) (bool, error) { } func editorConfigGlobRegexp(pattern string) (string, error) { + body, err := translateEditorConfigGlobSegment(pattern) + if err != nil { + return "", err + } + expression := "^" + body + "$" + if _, err := regexp.Compile(expression); err != nil { + return "", err + } + return expression, nil +} + +// translateEditorConfigGlobSegment converts an editorconfig glob into a regular +// expression body. In addition to *, **, and ?, it implements brace expansion: +// comma lists ({a,b,c}) become alternations and numeric ranges ({n..m}) expand +// to the integers in the range, matching the editorconfig specification. +func translateEditorConfigGlobSegment(pattern string) (string, error) { var builder strings.Builder - builder.WriteByte('^') for index := 0; index < len(pattern); { char := pattern[index] switch char { @@ -220,22 +235,132 @@ func editorConfigGlobRegexp(pattern string) (string, error) { continue } builder.WriteString("[^/]*") + index++ case '?': builder.WriteString("[^/]") - case '.', '+', '(', ')', '|', '^', '$', '{', '}', '[', ']', '\\': + index++ + case '{': + end := matchingEditorConfigBrace(pattern, index) + if end < 0 { + // An unbalanced brace is matched literally. + builder.WriteString("\\{") + index++ + continue + } + expansion, err := expandEditorConfigBrace(pattern[index+1 : end]) + if err != nil { + return "", err + } + builder.WriteString(expansion) + index = end + 1 + case '.', '+', '(', ')', '|', '^', '$', '}', '[', ']', '\\': builder.WriteByte('\\') builder.WriteByte(char) + index++ default: builder.WriteByte(char) + index++ } - index++ } - builder.WriteByte('$') - expression := builder.String() - if _, err := regexp.Compile(expression); err != nil { - return "", err + return builder.String(), nil +} + +// matchingEditorConfigBrace returns the index of the '}' that closes the '{' at +// open, accounting for nesting, or -1 when the braces are unbalanced. +func matchingEditorConfigBrace(pattern string, open int) int { + depth := 0 + for index := open; index < len(pattern); index++ { + switch pattern[index] { + case '{': + depth++ + case '}': + depth-- + if depth == 0 { + return index + } + } } - return expression, nil + return -1 +} + +// expandEditorConfigBrace turns the body of a brace expression into a regex +// fragment. A comma list becomes an alternation; a lone "n..m" body becomes a +// numeric range. A body with neither comma nor range is treated as literal +// braces, matching the editorconfig reference implementation. +func expandEditorConfigBrace(inner string) (string, error) { + options := splitTopLevelBraceCommas(inner) + if len(options) == 1 { + if rangeExpr, ok, err := editorConfigNumericRange(inner); err != nil { + return "", err + } else if ok { + return rangeExpr, nil + } + segment, err := translateEditorConfigGlobSegment(inner) + if err != nil { + return "", err + } + return "\\{" + segment + "\\}", nil + } + parts := make([]string, 0, len(options)) + for _, option := range options { + segment, err := translateEditorConfigGlobSegment(option) + if err != nil { + return "", err + } + parts = append(parts, segment) + } + return "(?:" + strings.Join(parts, "|") + ")", nil +} + +// splitTopLevelBraceCommas splits a brace body on commas that are not nested +// inside an inner brace group. +func splitTopLevelBraceCommas(inner string) []string { + parts := []string{} + depth := 0 + start := 0 + for index := 0; index < len(inner); index++ { + switch inner[index] { + case '{': + depth++ + case '}': + depth-- + case ',': + if depth == 0 { + parts = append(parts, inner[start:index]) + start = index + 1 + } + } + } + return append(parts, inner[start:]) +} + +// editorConfigNumericRange expands "n..m" into an alternation of the integers +// in [n, m]. It returns ok=false when the body is not a numeric range. +func editorConfigNumericRange(inner string) (string, bool, error) { + separator := strings.Index(inner, "..") + if separator < 0 { + return "", false, nil + } + low, err := strconv.Atoi(strings.TrimSpace(inner[:separator])) + if err != nil { + return "", false, nil + } + high, err := strconv.Atoi(strings.TrimSpace(inner[separator+2:])) + if err != nil { + return "", false, nil + } + if low > high { + low, high = high, low + } + const maxRangeSize = 65536 + if high-low >= maxRangeSize { + return "", false, fmt.Errorf("editorconfig numeric range {%s} is too large", inner) + } + parts := make([]string, 0, high-low+1) + for value := low; value <= high; value++ { + parts = append(parts, strconv.Itoa(value)) + } + return "(?:" + strings.Join(parts, "|") + ")", true, nil } func pathBase(path string) string { diff --git a/internal/topdata/editorconfig_glob_test.go b/internal/topdata/editorconfig_glob_test.go new file mode 100644 index 0000000..485646a --- /dev/null +++ b/internal/topdata/editorconfig_glob_test.go @@ -0,0 +1,56 @@ +package topdata + +import ( + "os" + "path/filepath" + "testing" +) + +func TestEditorConfigPatternMatchesBraceExpansion(t *testing.T) { + cases := []struct { + pattern string + path string + want bool + }{ + {"*.{json,jsonc}", "lock.json", true}, + {"*.{json,jsonc}", "types.jsonc", true}, + {"*.{json,jsonc}", "notes.md", false}, + {"*.{js,ts,tsx}", "main.tsx", true}, + {"{foo,bar}.txt", "bar.txt", true}, + {"{foo,bar}.txt", "baz.txt", false}, + {"page{1..3}.json", "page2.json", true}, + {"page{1..3}.json", "page4.json", false}, + // No comma and not a range: editorconfig treats braces literally. + {"{single}.txt", "{single}.txt", true}, + {"{single}.txt", "single.txt", false}, + } + for _, c := range cases { + got, err := editorConfigPatternMatches(c.pattern, c.path) + if err != nil { + t.Fatalf("pattern %q path %q: %v", c.pattern, c.path, err) + } + if got != c.want { + t.Errorf("editorConfigPatternMatches(%q, %q) = %v, want %v", c.pattern, c.path, got, c.want) + } + } +} + +func TestSaveLockfileHonorsBraceGlobIndentSize(t *testing.T) { + root := t.TempDir() + writeFile(t, filepath.Join(root, ".editorconfig"), "root = true\n\n[*]\nindent_style = space\nindent_size = 2\n\n[*.{json,jsonc}]\nindent_size = 4\n") + lockPath := filepath.Join(root, "data", "damagetypes", "registry", "lock.json") + mkdirAll(t, filepath.Dir(lockPath)) + writeFile(t, lockPath, "{\n \"damagetype:acid\": 4\n}\n") + + if err := saveLockfile(lockPath, map[string]int{"damagetype:acid": 4}); err != nil { + t.Fatalf("saveLockfile: %v", err) + } + got, err := os.ReadFile(lockPath) + if err != nil { + t.Fatalf("read lockfile: %v", err) + } + want := "{\n \"damagetype:acid\": 4\n}\n" + if string(got) != want { + t.Fatalf("expected 4-space indent resolved from [*.{json,jsonc}]; got:\n%q", string(got)) + } +} diff --git a/internal/topdata/itemprops_registry.go b/internal/topdata/itemprops_registry.go index d66bde9..1c948ec 100644 --- a/internal/topdata/itemprops_registry.go +++ b/internal/topdata/itemprops_registry.go @@ -166,7 +166,7 @@ func loadRegistryRows(path string) ([]map[string]any, error) { return rows, nil } -func collectItempropsRegistryDatasets(dataDir string) ([]nativeCollectedDataset, error) { +func collectItempropsRegistryDatasets(dataDir string, persistLocks bool) ([]nativeCollectedDataset, error) { registry, err := loadItempropsRegistry(dataDir) if err != nil { return nil, err @@ -188,7 +188,7 @@ func collectItempropsRegistryDatasets(dataDir string) ([]nativeCollectedDataset, return nil, err } lockModified = lockModified || costModified || paramModified - if lockModified { + if lockModified && persistLocks { if err := saveLockfile(registry.LockPath, registry.LockData); err != nil { return nil, err } diff --git a/internal/topdata/native.go b/internal/topdata/native.go index bee7548..22c44c6 100644 --- a/internal/topdata/native.go +++ b/internal/topdata/native.go @@ -398,7 +398,7 @@ func buildNativeUnchecked(p *project.Project, opts NativeBuildOptions, progress datasets = applyTopDataValueEncodings(datasets, p.Config.TopData.ValueEncodings) datasets = applyTopDataValueDefaults(datasets, p.Config.TopData.ValueDefaults) datasets = applyTopDataRowGeneration(datasets, p.Config.TopData.RowGeneration) - registryDatasets, err := collectGeneratedRegistryDatasets(dataDir) + registryDatasets, err := collectGeneratedRegistryDatasets(dataDir, true) if err != nil { return BuildResult{}, err } @@ -1073,7 +1073,7 @@ func discoverNativeOutputCatalog(dataDir string) (map[string]string, error) { if err != nil { return nil, err } - registryDatasets, err := collectGeneratedRegistryDatasets(dataDir) + registryDatasets, err := collectGeneratedRegistryDatasets(dataDir, false) if err != nil { return nil, err } diff --git a/internal/topdata/racialtypes_registry.go b/internal/topdata/racialtypes_registry.go index 1584d2a..e974bc2 100644 --- a/internal/topdata/racialtypes_registry.go +++ b/internal/topdata/racialtypes_registry.go @@ -120,7 +120,7 @@ func loadRacialtypesRegistryRaceRows(dir string) ([]map[string]any, error) { return rows, nil } -func collectRacialtypesRegistryDatasets(dataDir string) ([]nativeCollectedDataset, error) { +func collectRacialtypesRegistryDatasets(dataDir string, persistLocks bool) ([]nativeCollectedDataset, error) { registry, err := loadRacialtypesRegistry(dataDir) if err != nil { return nil, err @@ -138,7 +138,7 @@ func collectRacialtypesRegistryDatasets(dataDir string) ([]nativeCollectedDatase return nil, err } lockModified = lockModified || raceLockModified - if lockModified { + if lockModified && persistLocks { if err := saveLockfile(registry.LockPath, registry.LockData); err != nil { return nil, err } diff --git a/internal/topdata/registry_readonly_test.go b/internal/topdata/registry_readonly_test.go new file mode 100644 index 0000000..223e990 --- /dev/null +++ b/internal/topdata/registry_readonly_test.go @@ -0,0 +1,64 @@ +package topdata + +import ( + "os" + "path/filepath" + "testing" +) + +const damagetypesRegistryTestTypes = `{ + "rows": [ + {"id": 0, "key": "damagetype:bludgeoning", "label": "Bludgeoning", "group_label": "Physical", "damage_type_group": 0}, + {"id": 1, "key": "damagetype:piercing", "label": "Piercing", "group_label": "Physical", "damage_type_group": 0} + ] +} +` + +// validate-topdata must never modify source lockfiles. Collection in read-only +// mode must leave the registry lock byte-identical even though the rows carry +// explicit ids that would otherwise mark the lock "modified". +func TestCollectGeneratedRegistryDatasetsReadOnlyDoesNotWriteLock(t *testing.T) { + dataDir := t.TempDir() + regDir := filepath.Join(dataDir, "damagetypes", "registry") + mkdirAll(t, regDir) + writeFile(t, filepath.Join(regDir, "types.json"), damagetypesRegistryTestTypes) + + lockPath := filepath.Join(regDir, "lock.json") + original := "{\n \"damagetype:bludgeoning\": 0,\n \"damagetype:piercing\": 1\n}\n" + writeFile(t, lockPath, original) + + if _, err := collectGeneratedRegistryDatasets(dataDir, false); err != nil { + t.Fatalf("read-only collection failed: %v", err) + } + + got, err := os.ReadFile(lockPath) + if err != nil { + t.Fatalf("read lockfile: %v", err) + } + if string(got) != original { + t.Fatalf("read-only collection must not modify lockfile.\nwant: %q\ngot: %q", original, string(got)) + } +} + +// build-topdata still persists allocated ids to source lockfiles. +func TestCollectGeneratedRegistryDatasetsPersistWritesLock(t *testing.T) { + dataDir := t.TempDir() + regDir := filepath.Join(dataDir, "damagetypes", "registry") + mkdirAll(t, regDir) + writeFile(t, filepath.Join(regDir, "types.json"), damagetypesRegistryTestTypes) + + lockPath := filepath.Join(regDir, "lock.json") + writeFile(t, lockPath, "{\n \"damagetype:bludgeoning\": 0\n}\n") + + if _, err := collectGeneratedRegistryDatasets(dataDir, true); err != nil { + t.Fatalf("persisting collection failed: %v", err) + } + + lockData, err := loadLockfile(lockPath) + if err != nil { + t.Fatalf("load lockfile: %v", err) + } + if lockData["damagetype:piercing"] != 1 { + t.Fatalf("persisting collection should record allocated id; got %#v", lockData) + } +} diff --git a/internal/topdata/top_package.go b/internal/topdata/top_package.go index 313b829..76d9a85 100644 --- a/internal/topdata/top_package.go +++ b/internal/topdata/top_package.go @@ -557,7 +557,7 @@ func expectedCompiled2DAOutputs(p *project.Project) (map[string]struct{}, error) if err != nil { return nil, err } - registryDatasets, err := collectGeneratedRegistryDatasets(dataDir) + registryDatasets, err := collectGeneratedRegistryDatasets(dataDir, false) if err != nil { return nil, err } diff --git a/internal/topdata/topdata.go b/internal/topdata/topdata.go index 4059c59..a70e563 100644 --- a/internal/topdata/topdata.go +++ b/internal/topdata/topdata.go @@ -1320,7 +1320,7 @@ func validateNativeOutputCatalog(dataDir string, report *ValidationReport) { }) return } - registryDatasets, err := collectGeneratedRegistryDatasets(dataDir) + registryDatasets, err := collectGeneratedRegistryDatasets(dataDir, false) if err != nil { report.Diagnostics = append(report.Diagnostics, Diagnostic{ Severity: SeverityError, @@ -2307,7 +2307,7 @@ func validateNativeBuildability(p *project.Project, report *ValidationReport) { }) return } - registryDatasets, err := collectGeneratedRegistryDatasets(dataDir) + registryDatasets, err := collectGeneratedRegistryDatasets(dataDir, false) if err != nil { report.Diagnostics = append(report.Diagnostics, Diagnostic{ Severity: SeverityError, diff --git a/internal/topdata/wiki_native.go b/internal/topdata/wiki_native.go index 6a4a1c8..1169bec 100644 --- a/internal/topdata/wiki_native.go +++ b/internal/topdata/wiki_native.go @@ -748,7 +748,7 @@ func collectWikiRacialtypesDatasets(dataDir string, datasets []nativeDataset) ([ if len(out) > 0 { return out, nil } - return collectRacialtypesRegistryDatasets(dataDir) + return collectRacialtypesRegistryDatasets(dataDir, false) } func collectWikiRacialtypeStatuses(dataDir string) map[string]string {