From 89c1fcd8fae93a64d304251a35a65763818aa8b1 Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Mon, 14 Sep 2026 23:47:15 +0200 Subject: --json carries exclusions and matching warnings --- CHANGELOG.md | 2 ++ cmd/krino/matching_test.go | 49 ++++++++++++++++++++++++++++++++++++++++++++++ cmd/krino/sort.go | 16 ++++++++++++++- internal/plan/json.go | 49 ++++++++++++++++++++++++++++++++++++---------- internal/plan/json_test.go | 6 +++--- 5 files changed, 108 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1a6a53..2e09682 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ could change its answer; `explain` shows `?`. A document read only in part answers the keywords found in what was read, and leaves the others unknown. +- `--json` says which exclude set a file aside, carries the warnings raised + while matching, and lists unmatched files that raised one. - A literal `{{` or `}}` in a destination is text, not a placeholder: the directory a placeholder may not leave, and the directory left out of the walk, now include it. diff --git a/cmd/krino/matching_test.go b/cmd/krino/matching_test.go index 6986e99..754e577 100644 --- a/cmd/krino/matching_test.go +++ b/cmd/krino/matching_test.go @@ -499,3 +499,52 @@ func TestLongNameGetsItsOwnLine(t *testing.T) { t.Errorf("short name block:\n%s\nwant substring:\n%s", out, want) } } + +// TestDryRunJSONCarriesExclusionsAndWarnings: the JSON plan says which +// exclude set a file aside and carries the warnings matching raised, for +// matched and unmatched files alike, as the text plan does (triage 28h). +func TestDryRunJSONCarriesExclusionsAndWarnings(t *testing.T) { + h := home(t) + dl := filepath.Join(h, "dl") + os.MkdirAll(dl, 0o755) + old := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + for name, body := range map[string]string{"keep.pdf": "x", "big.txt": strings.Repeat("y", 4096)} { + p := filepath.Join(dl, name) + os.WriteFile(p, []byte(body), 0o644) + os.Chtimes(p, old, old) + } + if code, _, errOut := runCLI(t, "init"); code != 0 { + t.Fatal(errOut) + } + if code, _, errOut := runCLI(t, "new", "dl", dl); code != 0 { + t.Fatal(errOut) + } + rules := "(path \"~/dl\")\n(max-read 1K)\n(exclude (name \"^keep\"))\n(rule \"acme\" (when (content \"acme\")) (move \"Acme\"))\n" + os.WriteFile(filepath.Join(h, ".config", "krino", "dirs", "dl.conf"), []byte(rules), 0o644) + code, out, errOut := runCLI(t, "-n", "--json") + if code != 0 { + t.Fatalf("exit %d: %s", code, errOut) + } + var doc struct { + Dirs []struct { + Files []struct { + Rel string `json:"rel"` + Excluded string `json:"excluded"` + } `json:"files"` + Unmatched []struct { + Rel string `json:"rel"` + Warnings []string `json:"warnings"` + } `json:"unmatched"` + } `json:"dirs"` + } + if err := json.Unmarshal([]byte(out), &doc); err != nil { + t.Fatalf("%v\n%s", err, out) + } + d := doc.Dirs[0] + if len(d.Files) != 1 || d.Files[0].Rel != "keep.pdf" || d.Files[0].Excluded != `(exclude (name "^keep"))` { + t.Errorf("files = %+v; want keep.pdf excluded by its form\n%s", d.Files, out) + } + if len(d.Unmatched) != 1 || d.Unmatched[0].Rel != "big.txt" || len(d.Unmatched[0].Warnings) == 0 || !strings.Contains(d.Unmatched[0].Warnings[0], "content unreadable") { + t.Errorf("unmatched = %+v; want big.txt with its content warning\n%s", d.Unmatched, out) + } +} diff --git a/cmd/krino/sort.go b/cmd/krino/sort.go index f600475..83ede11 100644 --- a/cmd/krino/sort.go +++ b/cmd/krino/sort.go @@ -183,7 +183,7 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int { // --json is only ever reached with -n (checked above), and // Ruling 7 is explicit that JSON must never be paged, so // this returns before any of the paging/review code below. - jsonDirs = append(jsonDirs, plan.NewJSONDir(d.Name, d.Root, dp.Chains, dp.Result.Warnings)) + jsonDirs = append(jsonDirs, plan.NewJSONDir(d.Name, d.Root, dp.Chains, fileNotes(dp.Result), dp.Result.Warnings)) return false } @@ -427,6 +427,20 @@ type warnLine struct { // the chain-level warnings (e.g. "moved more than once"), keyed by // Chain.File.Rel; every chain's file is necessarily also in r.Matched (only // matched files ever reach plan.Build), so it is visited exactly once here. +// fileNotes is r's per-file findings - the exclude that set a file aside, +// the warnings matching raised - for the JSON plan. +func fileNotes(r *engine.Result) map[string]plan.FileNotes { + notes := map[string]plan.FileNotes{} + for _, fms := range [][]engine.FileMatch{r.Matched, r.Unmatched} { + for _, fm := range fms { + if fm.Excluded != "" || len(fm.Warnings) > 0 { + notes[fm.File.Rel] = plan.FileNotes{File: fm.File, Excluded: fm.Excluded, Warnings: fm.Warnings} + } + } + } + return notes +} + func collectWarnings(r *engine.Result, chains []plan.Chain) []warnLine { files := make([]engine.FileMatch, 0, len(r.Matched)+len(r.Unmatched)) files = append(files, r.Matched...) diff --git a/internal/plan/json.go b/internal/plan/json.go index 0b4f1bc..04b96ac 100644 --- a/internal/plan/json.go +++ b/internal/plan/json.go @@ -2,7 +2,12 @@ package plan -import "time" +import ( + "sort" + "time" + + "krino/internal/scan" +) // jsonNote is carried in every JSON document, warning readers that the // shape is not yet stable. @@ -18,10 +23,14 @@ type JSON struct { // JSONDir is one directory's plan. type JSONDir struct { - Name string `json:"name"` - Root string `json:"root"` - Files []JSONFile `json:"files"` - Warnings []string `json:"warnings,omitempty"` + Name string `json:"name"` + Root string `json:"root"` + Files []JSONFile `json:"files"` + // Unmatched holds the files no rule matched that still raised a + // warning (a content test that could not read them), as the text + // plan's warnings list does; other unmatched files are left out. + Unmatched []JSONFile `json:"unmatched,omitempty"` + Warnings []string `json:"warnings,omitempty"` } // JSONFile is one file's chain. @@ -29,8 +38,16 @@ type JSONFile struct { Rel string `json:"rel"` Size int64 `json:"size"` ModTime time.Time `json:"mtime"` + Excluded string `json:"excluded,omitempty"` // the exclude form that set the file aside Steps []JSONStep `json:"steps"` - Warnings []string `json:"warnings,omitempty"` + Warnings []string `json:"warnings,omitempty"` // raised while matching, then while planning +} + +// FileNotes is what matching found about one file besides its chain. +type FileNotes struct { + File scan.File + Excluded string + Warnings []string } // JSONStep is one step of a chain. D14: Reason is carried because the text @@ -69,10 +86,14 @@ func NewJSON(dirs []JSONDir) JSON { return JSON{Version: 1, Note: jsonNote, Dirs: dirs} } -// NewJSONDir converts one directory's chains. -func NewJSONDir(name, root string, chains []Chain, warnings []string) JSONDir { +// NewJSONDir converts one directory's chains. notes are matching's findings +// by Rel, for files with a chain and for unmatched files alike (triage 28h). +func NewJSONDir(name, root string, chains []Chain, notes map[string]FileNotes, warnings []string) JSONDir { files := make([]JSONFile, 0, len(chains)) + inPlan := make(map[string]bool, len(chains)) for _, ch := range chains { + inPlan[ch.File.Rel] = true + note := notes[ch.File.Rel] steps := make([]JSONStep, 0, len(ch.Steps)) for _, s := range ch.Steps { steps = append(steps, JSONStep{ @@ -89,9 +110,17 @@ func NewJSONDir(name, root string, chains []Chain, warnings []string) JSONDir { Rel: ch.File.Rel, Size: ch.File.Size, ModTime: ch.File.ModTime, + Excluded: note.Excluded, Steps: steps, - Warnings: ch.Warnings, + Warnings: append(append([]string(nil), note.Warnings...), ch.Warnings...), }) } - return JSONDir{Name: name, Root: root, Files: files, Warnings: warnings} + var unmatched []JSONFile + for rel, note := range notes { + if !inPlan[rel] && len(note.Warnings) > 0 { + unmatched = append(unmatched, JSONFile{Rel: rel, Size: note.File.Size, ModTime: note.File.ModTime, Steps: []JSONStep{}, Warnings: note.Warnings}) + } + } + sort.Slice(unmatched, func(i, j int) bool { return unmatched[i].Rel < unmatched[j].Rel }) + return JSONDir{Name: name, Root: root, Files: files, Unmatched: unmatched, Warnings: warnings} } diff --git a/internal/plan/json_test.go b/internal/plan/json_test.go index 86c181c..d67313e 100644 --- a/internal/plan/json_test.go +++ b/internal/plan/json_test.go @@ -21,7 +21,7 @@ func TestJSONDir(t *testing.T) { }, Warnings: []string{"moved more than once; a (stop) is probably missing"}, }} - b, err := json.MarshalIndent(JSON{Version: 1, Note: jsonNote, Dirs: []JSONDir{NewJSONDir("dl", "/r", chains, nil)}}, "", " ") + b, err := json.MarshalIndent(JSON{Version: 1, Note: jsonNote, Dirs: []JSONDir{NewJSONDir("dl", "/r", chains, nil, nil)}}, "", " ") if err != nil { t.Fatal(err) } @@ -61,7 +61,7 @@ func TestJSONDir(t *testing.T) { func TestJSONDirEmptyStepsAndFilesAreArraysNotNull(t *testing.T) { stepless := []Chain{{File: file("/r", "y.pdf")}} - dir := NewJSONDir("dl", "/r", stepless, nil) + dir := NewJSONDir("dl", "/r", stepless, nil, nil) b, err := json.Marshal(dir) if err != nil { t.Fatal(err) @@ -73,7 +73,7 @@ func TestJSONDirEmptyStepsAndFilesAreArraysNotNull(t *testing.T) { t.Errorf("stepless file's steps should marshal as []: %s", b) } - empty := NewJSONDir("dl", "/r", nil, nil) + empty := NewJSONDir("dl", "/r", nil, nil, nil) b, err = json.Marshal(empty) if err != nil { t.Fatal(err) -- cgit v1.3