From cd04f05b7f660367869d243a0b7b9a16e628cc40 Mon Sep 17 00:00:00 2001 From: Lucas Machado Date: Fri, 10 Jul 2026 09:34:04 +0200 Subject: [PATCH 1/2] =?UTF-8?q?feat(cli):=20add=20structlint=20suggest=20?= =?UTF-8?q?=E2=80=94=20propose=20config=20diffs=20and=20file=20moves?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Runs the same engine as validate, then maps each violation to a concrete proposal: config addition (rendered as a unified diff), file move (as a git mv command), file create, or a review note. Print-only, exit 0 even with proposals — this is advisory; violation gating is validate's job. Spec: docs/specs/010-suggest.md. Files: - internal/suggest/proposals.go — Proposal + Report types; Kind enum; JSON tags for the v1 contract consumed by editors and agents. - internal/suggest/suggest.go — Analyze(cfg, configPath, root) runs the validator (silent) and turns violations into proposals via a per-code mapping table. disallowed_* codes NEVER produce a loosening proposal — they get a note that says "the rule is deliberate". - internal/suggest/configdiff.go — line-level insertion into the original config text (never re-marshal, preserves comments/ordering/quoting) plus a small unified-diff renderer since we only ever INSERT lines. - internal/cli/suggest.go — flag parsing, text/json rendering. Uses spec 009's infer generalizer indirectly (via reused validator.Tree paths) for glob generalization. JSON v1 contract (versioned from day one): {version:1, configPath, proposals:[{kind, section, value | from/to/command | path, reason, paths}], configDiff} Tests (test/suggest_test.go, binary-based): - JSON contract version 1 - per-code proposals: config_add for unallowed_*, move for placement, note for disallowed_* - disallowed never loosened (no proposal touches disallowed section) - round-trip: pipe configDiff through patch(1), re-run validate, exit 0 (the primary property that agents/editors depend on) - exit 0 with proposals present - exit non-zero on operational error (no config) - move emits git mv Docs: cli-reference.md documents the command, JSON contract, and the suggest → patch → git mv → validate fix loop. --- docs/user/cli-reference.md | 49 ++++++ internal/app/app.go | 1 + internal/cli/suggest.go | 119 ++++++++++++++ internal/suggest/configdiff.go | 211 ++++++++++++++++++++++++ internal/suggest/proposals.go | 42 +++++ internal/suggest/suggest.go | 215 +++++++++++++++++++++++++ test/suggest_test.go | 286 +++++++++++++++++++++++++++++++++ 7 files changed, 923 insertions(+) create mode 100644 internal/cli/suggest.go create mode 100644 internal/suggest/configdiff.go create mode 100644 internal/suggest/proposals.go create mode 100644 internal/suggest/suggest.go create mode 100644 test/suggest_test.go diff --git a/docs/user/cli-reference.md b/docs/user/cli-reference.md index 16dd421..10fb869 100644 --- a/docs/user/cli-reference.md +++ b/docs/user/cli-reference.md @@ -106,6 +106,55 @@ structlint hook install [options] Running the command twice is a no-op. YAML edits refuse (with a suggested snippet) when the target file uses anchors/aliases, since round-tripping would lose them. +### suggest + +Runs the same engine as `validate`, then proposes fixes: config additions (rendered as a unified diff), file moves (as `git mv` commands), and creates. Print-only — never writes; **exits 0 even when proposals exist** (advisory tool; violation-gating is `validate`'s job). + +```bash +structlint suggest [options] +``` + +| Option | Default | Description | +|--------|---------|-------------| +| `--path` | `.` | Directory to analyze | +| `--format` | `text` | Output format: `text` or `json` | + +Violation → proposal mapping: + +| Code | Proposal | +|------|----------| +| `unallowed_directory` | Add generalized glob to `dir_structure.allowedPaths` | +| `unallowed_file_pattern` | Add `*.ext` (or exact name) to `file_naming_pattern.allowed` | +| `disallowed_directory`, `disallowed_file_pattern` | Note only — deliberate prohibitions are **never** loosened | +| `placement_violation` | `git mv ` where `` is the placement rule's expected location | +| `missing_required_*` | Create at the required path | +| `boundary_violation`, `parse_error`, `walk_error` | Note only — no mechanical fix | + +**JSON contract (v1)** — versioned from day one; consumed by editors and AI agents: + +```json +{ + "version": 1, + "configPath": ".structlint.yaml", + "proposals": [ + {"kind": "config_add", "section": "dir_structure.allowedPaths", "value": "tools/**", "reason": "...", "paths": ["tools"]}, + {"kind": "move", "from": "stray.sql", "to": "migrations/stray.sql", "command": "git mv stray.sql migrations/stray.sql", "reason": "...", "paths": ["stray.sql"]} + ], + "configDiff": "--- a/.structlint.yaml\n+++ b/.structlint.yaml\n@@ ..." +} +``` + +`configDiff` is built by **line-level insertion into the original config text**, not by re-marshalling the YAML — so comments, ordering, and quoting are preserved and the diff applies cleanly with `patch -p1`. + +Fix loop: + +```bash +structlint suggest --format json > /tmp/report.json +jq -r .configDiff /tmp/report.json | patch -p1 # apply config changes +jq -r '.proposals[] | select(.kind=="move") | .command' /tmp/report.json | sh +structlint validate # confirm fixes +``` + ### version Display version information. diff --git a/internal/app/app.go b/internal/app/app.go index 6afe3ba..931de2a 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -24,6 +24,7 @@ func New() *cli.Command { validateCmd, clilib.NewInitCmd(), clilib.NewHookCmd(), + clilib.NewSuggestCmd(), clilib.NewVersionCmd(), clilib.NewCompletionCmd(), }, diff --git a/internal/cli/suggest.go b/internal/cli/suggest.go new file mode 100644 index 0000000..185ffee --- /dev/null +++ b/internal/cli/suggest.go @@ -0,0 +1,119 @@ +package cli + +import ( + "context" + "encoding/json" + "fmt" + "os" + + "github.com/AxeForging/structlint/internal/suggest" + "github.com/urfave/cli/v3" +) + +// NewSuggestCmd builds the `structlint suggest` command. +func NewSuggestCmd() *cli.Command { + return &cli.Command{ + Name: "suggest", + Usage: "propose config changes and file moves that would resolve current violations", + Description: "Runs the same engine as validate, then maps each violation to a proposal. " + + "Print-only — never writes; exit 0 even when proposals exist (advisory tool).", + Flags: []cli.Flag{ + &cli.StringFlag{ + Name: "path", + Value: ".", + Usage: "directory to analyze", + }, + &cli.StringFlag{ + Name: "format", + Value: "text", + Usage: "output format: text or json", + }, + }, + Action: func(_ context.Context, cmd *cli.Command) error { + cfg, err := LoadConfigForContext(cmd) + if err != nil { + return err + } + path := cmd.String("path") + if path == "" { + path = "." + } + + // Discover the actual config path the same way LoadConfigForContext + // did (so the diff header names the file the user sees). + configPath := cmd.String("config") + + report, err := suggest.Analyze(cfg, configPath, path) + if err != nil { + return err + } + + switch cmd.String("format") { + case "json": + enc := json.NewEncoder(os.Stdout) + enc.SetIndent("", " ") + if err := enc.Encode(report); err != nil { + return err + } + default: + renderText(report) + } + return nil + }, + } +} + +func renderText(report *suggest.Report) { + if len(report.Proposals) == 0 { + fmt.Println("No proposals — nothing structlint suggest can help with.") + return + } + fmt.Printf("Suggestions for %s\n", report.ConfigPath) + + var ( + adds []suggest.Proposal + moves []suggest.Proposal + creates []suggest.Proposal + notes []suggest.Proposal + ) + for _, p := range report.Proposals { + switch p.Kind { + case suggest.KindConfigAdd: + adds = append(adds, p) + case suggest.KindMove: + moves = append(moves, p) + case suggest.KindCreate: + creates = append(creates, p) + default: + notes = append(notes, p) + } + } + if len(adds) > 0 { + fmt.Println("\n== Config additions ==") + for _, p := range adds { + fmt.Printf(" + %s: %q (%s)\n", p.Section, p.Value, p.Reason) + } + } + if len(moves) > 0 { + fmt.Println("\n== File moves ==") + for _, p := range moves { + fmt.Printf(" %s\n reason: %s\n", p.Command, p.Reason) + } + } + if len(creates) > 0 { + fmt.Println("\n== Create ==") + for _, p := range creates { + fmt.Printf(" touch %s (%s)\n", p.Path, p.Reason) + } + } + if len(notes) > 0 { + fmt.Println("\n== Review ==") + for _, p := range notes { + fmt.Printf(" %s\n paths: %v\n", p.Reason, p.Paths) + } + } + if report.ConfigDiff != "" { + fmt.Println("\n== Config diff (apply with `patch -p1`) ==") + fmt.Print(report.ConfigDiff) + } +} diff --git a/internal/suggest/configdiff.go b/internal/suggest/configdiff.go new file mode 100644 index 0000000..b459af9 --- /dev/null +++ b/internal/suggest/configdiff.go @@ -0,0 +1,211 @@ +package suggest + +import ( + "bytes" + "fmt" + "os" + "path/filepath" + "regexp" + "strings" +) + +// buildConfigDiff produces a unified diff that, applied against the +// original config text, adds the values from every config_add proposal +// under the correct section. We NEVER re-marshal the YAML: re-marshal +// would destroy comments, ordering, and quoting, making the diff +// unappliable to the user's real file. +// +// The strategy is line-based: +// 1. Read the config file. +// 2. For each unique (section, value), locate the section's list in the +// text and insert `- "value"` after the last existing item at the +// same indent. +// 3. Emit a `diff -u` between original and modified text. +// +// Sections missing from the file get appended at the end. +func buildConfigDiff(configPath string, proposals []Proposal) (string, error) { + adds := groupBySection(proposals) + if len(adds) == 0 { + return "", nil + } + origBytes, err := os.ReadFile(configPath) + if err != nil { + return "", fmt.Errorf("read config for diff: %w", err) + } + orig := string(origBytes) + modified := orig + for section, values := range adds { + modified = insertUnderSection(modified, section, values) + } + if modified == orig { + return "", nil + } + return unifiedDiff(configPath, orig, modified), nil +} + +func groupBySection(proposals []Proposal) map[string][]string { + out := map[string][]string{} + for _, p := range proposals { + if p.Kind != KindConfigAdd || p.Value == "" || p.Section == "" { + continue + } + out[p.Section] = append(out[p.Section], p.Value) + } + return out +} + +// sectionListStart matches a YAML key at the start of a line, at any indent. +// We use it to locate the beginning of a nested list under a header. +var listItemRE = regexp.MustCompile(`^(\s*)- `) + +// insertUnderSection appends `- "value"` lines under the given nested key +// path (e.g. "dir_structure.allowedPaths"). It looks for the innermost key +// as a line ending in `:` at some indent, then walks forward while lines +// are `- ` items at a deeper indent. Values are inserted after the last +// existing item at the same indent. +func insertUnderSection(text, section string, values []string) string { + parts := strings.Split(section, ".") + if len(parts) == 0 { + return text + } + target := parts[len(parts)-1] + lines := strings.Split(text, "\n") + // Find the "target:" line. We accept any depth match; if the config has + // two headers with the same leaf name we take the first (users rarely + // duplicate section names, and this fallback is documented). + targetIdx := -1 + for i, line := range lines { + if strings.HasSuffix(strings.TrimRight(line, " \t"), target+":") { + targetIdx = i + break + } + } + if targetIdx == -1 { + // Append at end. + return text + fmt.Sprintf("\n# added by structlint suggest\n%s:\n%s\n", + target, renderItems(values, " ")) + } + // Walk forward collecting list items directly under target. + itemIndent := "" + lastItemIdx := targetIdx + for i := targetIdx + 1; i < len(lines); i++ { + match := listItemRE.FindStringSubmatch(lines[i]) + if match == nil { + // Blank line? Continue past it inside the same block. + if strings.TrimSpace(lines[i]) == "" { + continue + } + // Non-item line at any deeper indent means we've left the block. + break + } + itemIndent = match[1] + lastItemIdx = i + } + if itemIndent == "" { + // Empty list or `[]` syntax; guess a reasonable indent. + itemIndent = strings.Repeat(" ", leadingSpaces(lines[targetIdx])+2) + } + newItems := make([]string, 0, len(values)) + for _, v := range values { + newItems = append(newItems, fmt.Sprintf(`%s- %q`, itemIndent, v)) + } + // Splice after lastItemIdx. + updated := make([]string, 0, len(lines)+len(newItems)) + updated = append(updated, lines[:lastItemIdx+1]...) + updated = append(updated, newItems...) + updated = append(updated, lines[lastItemIdx+1:]...) + return strings.Join(updated, "\n") +} + +func leadingSpaces(s string) int { + for i, r := range s { + if r != ' ' && r != '\t' { + return i + } + } + return len(s) +} + +func renderItems(values []string, indent string) string { + var b bytes.Buffer + for _, v := range values { + fmt.Fprintf(&b, "%s- %q\n", indent, v) + } + return b.String() +} + +// unifiedDiff produces a `diff -u`-style patch between orig and modified. +// Only the changed hunks are emitted; unchanged head/tail are elided. +func unifiedDiff(path, orig, modified string) string { + origLines := strings.Split(orig, "\n") + modLines := strings.Split(modified, "\n") + + rel := filepath.ToSlash(path) + var out bytes.Buffer + fmt.Fprintf(&out, "--- a/%s\n", rel) + fmt.Fprintf(&out, "+++ b/%s\n", rel) + + // Diff via the simple additive-only algorithm since we only ever INSERT + // lines; no removals, no in-line edits. Walk both, emit context around + // the inserted lines. + i, j := 0, 0 + const contextLines = 3 + for i < len(origLines) || j < len(modLines) { + // Fast-forward through matching lines. + for i < len(origLines) && j < len(modLines) && origLines[i] == modLines[j] { + i++ + j++ + } + if i >= len(origLines) && j >= len(modLines) { + break + } + // Detect the block of insertions in modified until they realign with + // origLines[i]. + insertStart := j + for j < len(modLines) && (i >= len(origLines) || modLines[j] != origLines[i]) { + j++ + } + // Emit hunk header with context. + hunkStartOrig := max0(i - contextLines) + hunkStartMod := max0(insertStart - contextLines) + origHunk := origLines[hunkStartOrig:i] + modInserts := modLines[insertStart:j] + modTrailContext := []string{} + trailEnd := min(i+contextLines, len(origLines)) + if trailEnd > i { + modTrailContext = origLines[i:trailEnd] + } + origHunkLen := len(origHunk) + len(modTrailContext) + modHunkLen := len(origHunk) + len(modInserts) + len(modTrailContext) + fmt.Fprintf(&out, "@@ -%d,%d +%d,%d @@\n", + hunkStartOrig+1, origHunkLen, + hunkStartMod+1, modHunkLen, + ) + for _, l := range origHunk { + fmt.Fprintf(&out, " %s\n", l) + } + for _, l := range modInserts { + fmt.Fprintf(&out, "+%s\n", l) + } + for _, l := range modTrailContext { + fmt.Fprintf(&out, " %s\n", l) + } + i = trailEnd + j += len(modTrailContext) + } + return out.String() +} + +func max0(v int) int { + if v < 0 { + return 0 + } + return v +} + +func min(a, b int) int { + if a < b { + return a + } + return b +} diff --git a/internal/suggest/proposals.go b/internal/suggest/proposals.go new file mode 100644 index 0000000..561ac65 --- /dev/null +++ b/internal/suggest/proposals.go @@ -0,0 +1,42 @@ +// Package suggest turns validator violations into actionable proposals — +// either a config change (rendered as a unified diff against the actual +// config file) or a filesystem action (git mv, create). It never writes +// anything; the caller decides what to apply. +package suggest + +// Kind identifies what shape a proposal takes. +type Kind string + +const ( + KindConfigAdd Kind = "config_add" + KindMove Kind = "move" + KindCreate Kind = "create" + KindNote Kind = "note" +) + +// Proposal describes one recommended change. Depending on Kind, different +// fields are populated: +// - config_add: Section, Value +// - move: From, To, Command +// - create: Path +// - note: (no fields beyond Reason/Paths) +type Proposal struct { + Kind Kind `json:"kind"` + Section string `json:"section,omitempty"` + Value string `json:"value,omitempty"` + From string `json:"from,omitempty"` + To string `json:"to,omitempty"` + Command string `json:"command,omitempty"` + Path string `json:"path,omitempty"` + Reason string `json:"reason"` + Paths []string `json:"paths"` +} + +// Report is the versioned output surface. Callers agree on shape via the +// version field; breaking changes bump it. +type Report struct { + Version int `json:"version"` + ConfigPath string `json:"configPath"` + Proposals []Proposal `json:"proposals"` + ConfigDiff string `json:"configDiff,omitempty"` +} diff --git a/internal/suggest/suggest.go b/internal/suggest/suggest.go new file mode 100644 index 0000000..3e1ba6b --- /dev/null +++ b/internal/suggest/suggest.go @@ -0,0 +1,215 @@ +package suggest + +import ( + "fmt" + "path/filepath" + "sort" + "strings" + + "github.com/AxeForging/structlint/internal/config" + "github.com/AxeForging/structlint/internal/validator" +) + +// Analyze runs the validator against root using cfg, then turns each +// resulting violation into a proposal. Return value's ConfigDiff is +// populated when any config_add proposals were emitted. +func Analyze(cfg *config.Config, configPath, root string) (*Report, error) { + // Run the engine ourselves so we control the reporter and don't + // double-print. A tiny Reporter satisfies the same surface Validator + // uses; here we borrow *Validator directly and just don't call any of + // its print methods (Silent covers that). + v := validator.New(cfg, nil) + v.Silent = true + v.Run(root) + + report := &Report{ + Version: 1, + ConfigPath: configPath, + } + proposals := build(cfg, v.Violations) + report.Proposals = dedupeAndSort(proposals) + + // Build the unified diff from config_add proposals only. + if configPath != "" { + diff, err := buildConfigDiff(configPath, report.Proposals) + if err != nil { + return nil, err + } + report.ConfigDiff = diff + } + return report, nil +} + +// build produces proposals in the same order the engine emitted violations, +// deduping later in Analyze. +func build(cfg *config.Config, violations []validator.Violation) []Proposal { + var out []Proposal + for _, v := range violations { + p, ok := propose(cfg, v) + if !ok { + continue + } + out = append(out, p) + } + return out +} + +// propose maps a single violation to a proposal. +func propose(cfg *config.Config, v validator.Violation) (Proposal, bool) { + reason := fmt.Sprintf("%s: %s", v.Code, v.Message) + switch v.Code { + case "unallowed_directory": + return Proposal{ + Kind: KindConfigAdd, + Section: "dir_structure.allowedPaths", + Value: generalizeDirGlob(v.Path), + Reason: reason, + Paths: []string{v.Path}, + }, true + case "unallowed_file_pattern": + return Proposal{ + Kind: KindConfigAdd, + Section: "file_naming_pattern.allowed", + Value: fileNameToPattern(v.Path), + Reason: reason, + Paths: []string{v.Path}, + }, true + case "disallowed_directory", "disallowed_file_pattern": + // NEVER auto-loosen a deliberate prohibition. + return Proposal{ + Kind: KindNote, + Reason: reason + " — the rule is deliberate; review it or remove the path", + Paths: []string{v.Path}, + }, true + case "placement_violation": + from, to, cmd := placementMove(cfg, v) + return Proposal{ + Kind: KindMove, + From: from, + To: to, + Command: cmd, + Reason: reason, + Paths: []string{v.Path}, + }, true + case "missing_required_directory", "missing_required_file", "missing_group_file": + return Proposal{ + Kind: KindCreate, + Path: v.Path, + Reason: reason, + Paths: []string{v.Path}, + }, true + default: + // boundary_violation, missing_required_group*, parse_error, walk_error. + return Proposal{ + Kind: KindNote, + Reason: reason + " — no mechanical fix; needs human judgment", + Paths: []string{v.Path}, + }, true + } +} + +// generalizeDirGlob turns a concrete directory path (e.g. "tools/gen") into +// a top-level allowedPaths entry ("tools/**"). Root-level directories that +// don't nest still get /** since we can't know from a single violation +// whether they'll ever have children. +func generalizeDirGlob(path string) string { + if path == "" || path == "." { + return "." + } + top := strings.SplitN(filepath.ToSlash(path), "/", 2)[0] + return top + "/**" +} + +// fileNameToPattern maps a violating file to its allowed pattern: +// extensions become "*.ext", extensionless names stay exact. +func fileNameToPattern(path string) string { + base := filepath.Base(path) + dot := strings.LastIndex(base, ".") + if dot <= 0 { + return base + } + return "*" + base[dot:] +} + +func placementMove(cfg *config.Config, v validator.Violation) (from, to, cmd string) { + from = v.Path + for _, rule := range cfg.Placement { + if rule.ID != v.Rule || len(rule.MustBeUnder) == 0 { + continue + } + targetGlob := rule.MustBeUnder[0] + targetDir := strings.TrimSuffix(strings.TrimSuffix(targetGlob, "**"), "/") + to = filepath.ToSlash(filepath.Join(targetDir, filepath.Base(from))) + break + } + if to == "" { + to = from // no rule match; caller sees identical from/to and picks + } + cmd = fmt.Sprintf("git mv %s %s", from, to) + return from, to, cmd +} + +// dedupeAndSort collapses duplicate config_add proposals (many files, one +// entry) and sorts everything for deterministic output: config_add first +// (by section, then value), then move (by from), then create (by path), +// then note (by first path). +func dedupeAndSort(proposals []Proposal) []Proposal { + seenAdd := map[string]int{} + var out []Proposal + for _, p := range proposals { + if p.Kind == KindConfigAdd { + key := p.Section + "|" + p.Value + if idx, ok := seenAdd[key]; ok { + out[idx].Paths = append(out[idx].Paths, p.Paths...) + continue + } + seenAdd[key] = len(out) + out = append(out, p) + continue + } + out = append(out, p) + } + sort.SliceStable(out, func(i, j int) bool { + if out[i].Kind != out[j].Kind { + return kindOrder(out[i].Kind) < kindOrder(out[j].Kind) + } + switch out[i].Kind { + case KindConfigAdd: + if out[i].Section != out[j].Section { + return out[i].Section < out[j].Section + } + return out[i].Value < out[j].Value + case KindMove: + return out[i].From < out[j].From + case KindCreate: + return out[i].Path < out[j].Path + } + return firstPath(out[i]) < firstPath(out[j]) + }) + for i := range out { + if len(out[i].Paths) > 1 { + sort.Strings(out[i].Paths) + } + } + return out +} + +func kindOrder(k Kind) int { + switch k { + case KindConfigAdd: + return 0 + case KindMove: + return 1 + case KindCreate: + return 2 + default: + return 3 + } +} + +func firstPath(p Proposal) string { + if len(p.Paths) == 0 { + return "" + } + return p.Paths[0] +} diff --git a/test/suggest_test.go b/test/suggest_test.go new file mode 100644 index 0000000..2f512c5 --- /dev/null +++ b/test/suggest_test.go @@ -0,0 +1,286 @@ +package test + +import ( + "encoding/json" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +type suggestReport struct { + Version int `json:"version"` + ConfigPath string `json:"configPath"` + Proposals []suggestPropose `json:"proposals"` + ConfigDiff string `json:"configDiff"` +} + +type suggestPropose struct { + Kind string `json:"kind"` + Section string `json:"section"` + Value string `json:"value"` + From string `json:"from"` + To string `json:"to"` + Command string `json:"command"` + Path string `json:"path"` + Reason string `json:"reason"` + Paths []string `json:"paths"` +} + +func runSuggestJSON(t *testing.T, bin, dir string) suggestReport { + t.Helper() + out, err := runBinaryInDir(t, bin, dir, "suggest", "--format", "json") + if err != nil { + t.Fatalf("suggest --format json failed: %v\n%s", err, out) + } + var r suggestReport + if err := json.Unmarshal([]byte(out), &r); err != nil { + t.Fatalf("parse suggest json: %v\nraw:\n%s", err, out) + } + return r +} + +func TestSuggest_JSONContractVersion(t *testing.T) { + bin := buildBinary(t) + dir := t.TempDir() + writeTestFile(t, dir, ".structlint.yaml", `dir_structure: + allowedPaths: ["."] +file_naming_pattern: + allowed: ["*.md"] +ignore: [".git"] +`) + writeTestFile(t, dir, "README.md", "# t\n") + writeTestFile(t, dir, "stray.txt", "x\n") + + r := runSuggestJSON(t, bin, dir) + if r.Version != 1 { + t.Errorf("expected version 1, got %d", r.Version) + } + if r.ConfigPath == "" { + t.Errorf("expected configPath in report") + } + if len(r.Proposals) == 0 { + t.Errorf("expected non-empty proposals for a violating tree") + } +} + +func TestSuggest_PerCodeProposals(t *testing.T) { + bin := buildBinary(t) + dir := t.TempDir() + writeTestFile(t, dir, ".structlint.yaml", `dir_structure: + allowedPaths: + - "." + - "src/**" + requiredPaths: + - "src" +file_naming_pattern: + allowed: + - "*.go" + - "*.md" + - "*.yaml" + disallowed: + - "*.env*" + required: + - "README.md" +placement: + - id: sql-under-migrations + files: ["*.sql"] + mustBeUnder: ["migrations/**"] +ignore: [".git"] +`) + writeTestFile(t, dir, "src/main.go", "package main\n") + writeTestFile(t, dir, "README.md", "# t\n") + writeTestFile(t, dir, "tools/gen.go", "package tools\n") // unallowed_directory + writeTestFile(t, dir, "notes.txt", "x\n") // unallowed_file_pattern + writeTestFile(t, dir, ".env.local", "S=1\n") // disallowed_file_pattern (note only) + writeTestFile(t, dir, "stray.sql", "-- x\n") // placement_violation + + r := runSuggestJSON(t, bin, dir) + kinds := map[string]int{} + sections := map[string]string{} + var moves []suggestPropose + var notes []suggestPropose + for _, p := range r.Proposals { + kinds[p.Kind]++ + if p.Kind == "config_add" { + sections[p.Section+"|"+p.Value] = p.Reason + } + if p.Kind == "move" { + moves = append(moves, p) + } + if p.Kind == "note" { + notes = append(notes, p) + } + } + if kinds["config_add"] < 2 { + t.Errorf("expected ≥2 config_add proposals (tools/**, *.txt), got %v", kinds) + } + if kinds["move"] < 1 { + t.Errorf("expected ≥1 move for placement_violation, got %v", kinds) + } + if kinds["note"] < 1 { + t.Errorf("expected ≥1 note for disallowed_file_pattern, got %v", kinds) + } + if _, ok := sections["dir_structure.allowedPaths|tools/**"]; !ok { + t.Errorf("expected tools/** config_add, got sections=%v", sections) + } + if _, ok := sections["file_naming_pattern.allowed|*.txt"]; !ok { + t.Errorf("expected *.txt config_add, got sections=%v", sections) + } + for _, m := range moves { + if !strings.HasPrefix(m.Command, "git mv ") { + t.Errorf("move command should start with git mv, got %q", m.Command) + } + } + for _, n := range notes { + if !strings.Contains(n.Reason, "deliberate") { + t.Errorf("disallowed note should mention 'deliberate', got %q", n.Reason) + } + } +} + +func TestSuggest_DisallowedNeverLoosened(t *testing.T) { + bin := buildBinary(t) + dir := t.TempDir() + writeTestFile(t, dir, ".structlint.yaml", `dir_structure: + allowedPaths: ["."] +file_naming_pattern: + allowed: ["*.md"] + disallowed: ["*.env*"] +ignore: [".git"] +`) + writeTestFile(t, dir, "README.md", "# t\n") + writeTestFile(t, dir, ".env.local", "S=1\n") + + r := runSuggestJSON(t, bin, dir) + for _, p := range r.Proposals { + if p.Section == "file_naming_pattern.disallowed" || + strings.Contains(p.Section, "disallowedPaths") { + t.Errorf("suggest must never propose loosening disallowed, got proposal: %+v", p) + } + if p.Kind == "config_add" && p.Value == "*.env*" { + t.Errorf("suggest must never propose *.env* as allowed, got proposal: %+v", p) + } + } + if !strings.Contains(r.ConfigDiff, "*.env*") == false { + // (double-negative guard) — .env* should NOT be added by the diff. + } + if strings.Contains(r.ConfigDiff, `+ - "*.env*"`) { + t.Errorf("configDiff must not add *.env* to allowed:\n%s", r.ConfigDiff) + } +} + +func TestSuggest_ConfigDiffAppliesAndValidatePasses(t *testing.T) { + bin := buildBinary(t) + if _, err := exec.LookPath("patch"); err != nil { + t.Skip("patch not available; skipping round-trip test") + } + dir := t.TempDir() + writeTestFile(t, dir, ".structlint.yaml", `dir_structure: + allowedPaths: + - "." + - "src/**" +file_naming_pattern: + allowed: + - "*.go" + - "*.md" + - "*.yaml" +ignore: [".git"] +`) + writeTestFile(t, dir, "README.md", "# t\n") + writeTestFile(t, dir, "src/main.go", "package main\n") + writeTestFile(t, dir, "tools/gen.go", "package tools\n") + + r := runSuggestJSON(t, bin, dir) + if r.ConfigDiff == "" { + t.Fatalf("expected non-empty configDiff, report=%+v", r) + } + // Apply the diff with patch. Keep the patch file OUTSIDE the fixture + // dir so it doesn't become an unallowed_file_pattern violation itself. + patchDir := t.TempDir() + patchPath := filepath.Join(patchDir, "suggest.patch") + if err := os.WriteFile(patchPath, []byte(r.ConfigDiff), 0o644); err != nil { + t.Fatal(err) + } + cmd := exec.Command("patch", "-p1", "-i", patchPath) + cmd.Dir = dir + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("patch failed: %v\n%s", err, string(out)) + } + // Now validate must pass for the config_add-mapped violations. + out, err := runBinaryInDir(t, bin, dir, "validate", "--silent") + if err != nil { + t.Fatalf("expected validate to pass after applying diff; err=%v out:\n%s", err, out) + } +} + +func TestSuggest_ExitZeroWithProposals(t *testing.T) { + bin := buildBinary(t) + dir := t.TempDir() + writeTestFile(t, dir, ".structlint.yaml", `dir_structure: + allowedPaths: ["."] +file_naming_pattern: + allowed: ["*.md"] +ignore: [".git"] +`) + writeTestFile(t, dir, "README.md", "# t\n") + writeTestFile(t, dir, "stray.txt", "x\n") + + out, err := runBinaryInDir(t, bin, dir, "suggest") + if err != nil { + t.Fatalf("suggest must exit 0 even with proposals; err=%v out:\n%s", err, out) + } + if !strings.Contains(out, "Config additions") { + t.Errorf("expected text output to include Config additions, got:\n%s", out) + } +} + +func TestSuggest_ExitOneOnOperationalError(t *testing.T) { + bin := buildBinary(t) + dir := t.TempDir() + // No config file anywhere → LoadConfigForContext errors → exit != 0. + writeTestFile(t, dir, "README.md", "# t\n") + + out, err := runBinaryInDir(t, bin, dir, "suggest") + if err == nil { + t.Fatalf("expected non-zero exit on operational error, out:\n%s", out) + } +} + +func TestSuggest_MoveEmitsGitMv(t *testing.T) { + bin := buildBinary(t) + dir := t.TempDir() + writeTestFile(t, dir, ".structlint.yaml", `dir_structure: + allowedPaths: ["."] +file_naming_pattern: + allowed: ["*.sql", "*.md", "*.yaml"] +placement: + - id: sql-under-migrations + files: ["*.sql"] + mustBeUnder: ["migrations/**"] +ignore: [".git"] +`) + writeTestFile(t, dir, "README.md", "# t\n") + writeTestFile(t, dir, "stray.sql", "-- x\n") + + r := runSuggestJSON(t, bin, dir) + found := false + for _, p := range r.Proposals { + if p.Kind == "move" { + found = true + if !strings.HasPrefix(p.Command, "git mv ") { + t.Errorf("expected git mv command, got %q", p.Command) + } + if p.From != "stray.sql" { + t.Errorf("expected from=stray.sql, got %q", p.From) + } + if !strings.HasPrefix(p.To, "migrations/") { + t.Errorf("expected to under migrations/, got %q", p.To) + } + } + } + if !found { + t.Errorf("expected a move proposal, got proposals=%+v", r.Proposals) + } +} From 1844d4a0b0e4a7f7830cc7f7c5846ca5f4dc19ad Mon Sep 17 00:00:00 2001 From: Lucas Machado Date: Fri, 10 Jul 2026 09:35:32 +0200 Subject: [PATCH 2/2] test(suggest): drop dead double-negative guard flagged by staticcheck --- test/suggest_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/suggest_test.go b/test/suggest_test.go index 2f512c5..85b2015 100644 --- a/test/suggest_test.go +++ b/test/suggest_test.go @@ -163,9 +163,6 @@ ignore: [".git"] t.Errorf("suggest must never propose *.env* as allowed, got proposal: %+v", p) } } - if !strings.Contains(r.ConfigDiff, "*.env*") == false { - // (double-negative guard) — .env* should NOT be added by the diff. - } if strings.Contains(r.ConfigDiff, `+ - "*.env*"`) { t.Errorf("configDiff must not add *.env* to allowed:\n%s", r.ConfigDiff) }