refactor(validator): add rule engine, registry, and Tree snapshot#17
Merged
Conversation
Introduces the Tree/Rule/Registry surface that specs 009 (init --infer) and 010 (suggest) depend on. The seven Validate* methods are now enumerable through Registry() and driven by a single Validator.Run(path) entry point that snapshots the tree once. Spec: docs/specs/005-rule-engine.md. Scope (delivered): - internal/validator/walk.go: Snapshot() produces an ignore-filtered Tree with Entry list and O(1) file/dir lookups. - internal/validator/engine.go: Rule interface, RunContext, Registry(cfg), and Validator.Run(path). Skip predicate absorbs spec 001's staged / changed-only logic (dir-scope for dirs, file-scope for files). - internal/cli/validate.go: swaps seven Validate* calls for v.Run(path). - Parity fixtures under test/testdata/parity/ + goldens captured from the pre-refactor binary; test/engine_parity_test.go asserts text, JSON, and exit-code parity byte-for-byte across the fixtures. Deferred as follow-up: - Per-rule single-walk conversion (each rule's Validate* still runs its own filepath.Walk today; the goldens ensure any future conversion stays parity-safe). This iteration's win is architectural — rules are now enumerable, and callers like suggest can consume the Tree. Housekeeping to keep the test suite green with new fixtures: - .structlint.yaml ignore += test/testdata (deliberate rule-violating fixtures live under test/testdata/parity). - Existing project_validation_test.go and self_validation_test.go teach their hardcoded configs / walks to skip testdata too.
StructLint — All checks passed119 rules validated against
|
3 tasks
machado144
added a commit
that referenced
this pull request
Jul 10, 2026
## Summary Follow-up PR that honestly delivers what the roadmap PRs oversold. Everything gated by broadened parity goldens — 21 subtests across 6 fixtures, all byte-identical. ## What actually changed ### 1. True single-walk rule engine (the fix for spec 005) The prior "rule engine" (#17) shipped an architectural wrapper but kept the 7 per-rule \`filepath.Walk\` calls — every rule wrapper called the old \`Validate*\` method which walked the tree itself. Net cost: **8 walks per run** instead of the previous 7. The commit title lied. This does the actual conversion: - Each rule iterates \`ctx.Tree.Entries\` in walk order. - \`dirStructureRule\` tracks a \`skipTracker\` to preserve \`filepath.SkipDir\` semantics (disallowed subtree suppresses descendant checks — locked by the new \`disallowed-subtree\` fixture). - \`requiredPathsRule\` and \`requiredGroupsRule\` keep their \`os.Stat\` / glob helpers — legacy quirk of seeing through ignored directories is a documented parity requirement. - \`placementRule\` and \`boundariesRule\` preserve their \`Successes\` counter quirks. - Exported \`Validate*\` methods stay as thin wrappers via a new \`runSingleRule\` helper so the root \`validator_test.go\` and any external callers keep working. **Broadened parity fixtures**: \`boundaries\` (Go import parsing, module resolution), \`required-groups\` (\`eachDirMatching\` + \`oneOf\`), \`disallowed-subtree\` (SkipDir semantics with nested subtrees). Three cases the earlier goldens didn't touch at all. ### 2. Init templates deduped Deletes the 220-line \`projectTemplates\` map in \`internal/cli/init.go\`. \`init --type <T>\` now reads the same embedded preset files that \`extends: <T>-standard\` resolves against, via a new \`config.ReadPreset\` API, and prepends a one-line project-typed header. - \`init.go\`: 322 → 168 lines. - New \`test/init_presets_test.go\` proves init output body is byte-identical to the preset AND that \`init --type go\` vs \`extends: go-standard\` produce equivalent validation on the same tree. ### 3. Version pragma is actually enforced The prior \"mitigation\" for old-binary/new-config was docs suggesting \`# requires structlint >= vX.Y\`. Nothing checked it. Users saw the raw yaml \`field not found\` error. New \`internal/config/versioncheck.go\` parses the pragma from raw bytes **before** strict parsing and prints: \`\`\` .structlint.yaml requires structlint >= v0.6.0, but running version is v0.5.0. Upgrade the binary (go install github.com/AxeForging/structlint/cmd/structlint@latest) … \`\`\` Because the check runs before strict parsing, it works on binaries too old to know about \`extends\`. Dev builds (unstamped) skip the check. - 10 in-package unit tests over parse/compare edges. - 2 binary-based end-to-end tests that build pinned binaries with ldflags-injected \`Version\` values and prove the CLI surfaces the error cleanly. ### 4. Deeper SKILL.md validation \`skill_contract_test.go\` had shallow checks. \`skill_deep_test.go\` adds: - Frontmatter parses as real YAML with expected shape. - Trigger phrases survive block-scalar folding (parse YAML → normalize whitespace → search). - Every subcommand referenced in setup recipes actually boots via \`app.Run(--help)\`. - JSON v1 contract mentioned + all four exit codes documented. ## Numbers - 167 tests before → **191 tests after**. - \`init.go\`: 322 → 168 lines. - \`validator.go\`: 494 → 232 lines (all the walk bodies moved into engine.go's rules). - 6 parity fixtures with text/json/exit goldens. Zero drift. ## Test plan - [x] \`go test -race ./...\` — 191 pass. - [x] \`make build && ./bin/structlint validate\` — self-dogfood, exit 0. - [x] \`gofumpt -l .\` clean, \`go vet ./...\` clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Scope decision
Delivered: the architectural surface — Snapshot, Rule interface, Registry, RunContext, `v.Run`. Callers (spec 010's `suggest`) can now enumerate rules and consume a single `Tree`.
Deferred: collapsing the seven per-rule `filepath.Walk` calls into one. Each rule still executes its old body via a thin adapter. The perf win is a follow-up refactor; the parity goldens guarantee it stays parity-safe when it lands. Given the risk of regressing output on a widely-used validator and the fact that specs 009/010 only need the architectural surface, splitting the perf work out is the low-risk call.
Parity harness
Three fixtures cover the interesting cases:
Goldens capture text, `--format json`, and exit code. Regenerate with `bash test/testdata/parity/capture.sh /path/to/binary`.
Test plan