From 24a84671ace373ae331fa83a1ff484990f4dff0e Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Sat, 12 Sep 2026 12:58:14 +0200 Subject: krino: planning — chains, placeholders, conflicts, JSON MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/cond/compile.go | 22 ++++ internal/cond/compile_test.go | 17 +++ internal/cond/types.go | 12 ++ internal/dup/dup.go | 41 +++++++ internal/dup/dup_test.go | 23 ++++ internal/engine/engine.go | 38 +++++++ internal/engine/engine_test.go | 49 +++++++++ internal/engine/facts.go | 13 +-- internal/engine/match.go | 10 +- internal/engine/plan.go | 60 ++++++++++ internal/engine/plan_test.go | 146 +++++++++++++++++++++++++ internal/plan/chain.go | 222 +++++++++++++++++++++++++++++++++++++ internal/plan/chain_test.go | 143 ++++++++++++++++++++++++ internal/plan/conflict.go | 134 +++++++++++++++++++++++ internal/plan/conflict_test.go | 224 ++++++++++++++++++++++++++++++++++++++ internal/plan/index.go | 44 ++++++++ internal/plan/index_test.go | 28 +++++ internal/plan/json.go | 97 +++++++++++++++++ internal/plan/json_test.go | 87 +++++++++++++++ internal/plan/placeholder.go | 155 ++++++++++++++++++++++++++ internal/plan/placeholder_test.go | 94 ++++++++++++++++ internal/plan/step.go | 69 ++++++++++++ 22 files changed, 1715 insertions(+), 13 deletions(-) create mode 100644 internal/engine/plan.go create mode 100644 internal/engine/plan_test.go create mode 100644 internal/plan/chain.go create mode 100644 internal/plan/chain_test.go create mode 100644 internal/plan/conflict.go create mode 100644 internal/plan/conflict_test.go create mode 100644 internal/plan/index.go create mode 100644 internal/plan/index_test.go create mode 100644 internal/plan/json.go create mode 100644 internal/plan/json_test.go create mode 100644 internal/plan/placeholder.go create mode 100644 internal/plan/placeholder_test.go create mode 100644 internal/plan/step.go (limited to 'internal') diff --git a/internal/cond/compile.go b/internal/cond/compile.go index 4fdc75f..590d61a 100644 --- a/internal/cond/compile.go +++ b/internal/cond/compile.go @@ -362,6 +362,28 @@ func (c *compiler) compileMatched(n *sexp.Node) *node { return &node{kind: kMatched, pos: n.Pos, label: "matched", cost: costCheap} } +// collectNameGroups walks n and its children — and and or included, not +// excluded — appending the capture-group count of every pattern of every +// kName node reachable without crossing a not, in compile order. B1: a +// name test under a not never supplies Result.Captures (eval.go's negated +// tracking takes captures only when !negated), so it must not count toward +// checkCaptures' "does some name test in this rule have enough groups" +// either - a rule combining a capturing name test with an unrelated +// (not (name ...)) must still pass. +func collectNameGroups(n *node, out *[]int) { + if n == nil || n.kind == kNot { + return + } + if n.kind == kName { + for _, p := range n.patterns { + *out = append(*out, p.re.NumSubexp()) + } + } + for _, ch := range n.children { + collectNameGroups(ch, out) + } +} + // quotedExampleErr records the shared "takes X in quotes: write (test // "arg")" diagnostic used by name, path, content and duplicate. func (c *compiler) quotedExampleErr(a *sexp.Node, test, noun string) { diff --git a/internal/cond/compile_test.go b/internal/cond/compile_test.go index e68a4d8..2a07ad6 100644 --- a/internal/cond/compile_test.go +++ b/internal/cond/compile_test.go @@ -124,6 +124,23 @@ func TestEmptyChildrenGuard(t *testing.T) { } } +// TestNameGroups: B1b - NameGroups() reports the capture-group count of +// every name test reachable without crossing a not, and skips one reachable +// only under a not (B1): here the outer (name ...) has two groups and the +// (not (name ...)) one has one, but the result must carry only the outer's +// count. +func TestNameGroups(t *testing.T) { + c, errs := Compile("d.conf", nodes(t, `(and (name "inv-(\d+)-(\d+)") (not (name "draft-(\d+)")))`), Options{}) + if len(errs) > 0 { + t.Fatal(errs) + } + got := c.NameGroups() + want := []int{2} + if !reflect.DeepEqual(got, want) { + t.Errorf("NameGroups() = %v, want %v", got, want) + } +} + func TestGroupsMatchSpec(t *testing.T) { want := map[string]string{ "image": "jpg jpeg png gif webp bmp tif tiff heic heif avif svg ico raw cr2 nef arw dng", diff --git a/internal/cond/types.go b/internal/cond/types.go index 81c2f45..9455f63 100644 --- a/internal/cond/types.go +++ b/internal/cond/types.go @@ -93,6 +93,18 @@ type node struct { dirs []string } +// NameGroups returns the number of capture groups of every name test in the +// condition that can ever supply captures, in compile order: a name test +// nested inside and/or counts however deep, but one inside a not does not +// (B1) - it can never be the source of Result.Captures, so it must not be +// asked to justify a rule's use of {N} either. Empty when the rule has no +// such name test. +func (c *Cond) NameGroups() []int { + var out []int + collectNameGroups(c.root, &out) + return out +} + // groups maps a (type ...) group name to the extensions it expands to, // spec Appendix A. var groups = map[string][]string{ diff --git a/internal/dup/dup.go b/internal/dup/dup.go index 502ef31..e3c7424 100644 --- a/internal/dup/dup.go +++ b/internal/dup/dup.go @@ -381,6 +381,47 @@ func readAt(f *os.File, off int64) ([]byte, error) { return buf[:n], nil } +// SameContent reports whether a and b hold identical content: a stat and +// size check first, then the same partial/full hash comparison Lookup uses +// for scanned candidates. Neither file needs to have been scanned or +// indexed; this is the one place content identity is decided, so callers +// outside this package must not hash a second way. +func SameContent(a, b string) (bool, error) { + ai, err := os.Stat(a) + if err != nil { + return false, err + } + bi, err := os.Stat(b) + if err != nil { + return false, err + } + if ai.Size() != bi.Size() { + return false, nil + } + + aPartial, err := computePartialHash(a) + if err != nil { + return false, err + } + bPartial, err := computePartialHash(b) + if err != nil { + return false, err + } + if aPartial != bPartial { + return false, nil + } + + aFull, err := computeFullHash(a) + if err != nil { + return false, err + } + bFull, err := computeFullHash(b) + if err != nil { + return false, err + } + return aFull == bFull, nil +} + // computeFullHash hashes the whole file. func computeFullHash(path string) ([sha256.Size]byte, error) { f, err := os.Open(path) diff --git a/internal/dup/dup_test.go b/internal/dup/dup_test.go index fb4e64d..5d2621c 100644 --- a/internal/dup/dup_test.go +++ b/internal/dup/dup_test.go @@ -262,6 +262,29 @@ func TestLookupFailsWhenSubjectUnreadable(t *testing.T) { } } +// TestSameContentEqualSizeDifferentContent: two files of identical size but +// different bytes must not be reported as the same content — the partial +// hash (not just the size check) has to separate them. +func TestSameContentEqualSizeDifferentContent(t *testing.T) { + d := t.TempDir() + a := filepath.Join(d, "a.bin") + b := filepath.Join(d, "b.bin") + one := []byte("acme-invoice-01") + two := []byte("acme-invoice-02") + if len(one) != len(two) { + t.Fatal("fixture bug: files must be the same size") + } + if err := os.WriteFile(a, one, 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(b, two, 0o644); err != nil { + t.Fatal(err) + } + if ok, err := SameContent(a, b); err != nil || ok { + t.Errorf("SameContent(a, b) = %v, %v; want false, nil", ok, err) + } +} + // fakeDirEntry is an fs.DirEntry whose Info() returns a canned result, for // exercising addEntry's Info()-failure handling directly (A3) — a real // filepath.WalkDir gives no hook to inject a stat failure deterministically diff --git a/internal/engine/engine.go b/internal/engine/engine.go index eec9a60..ea61e8c 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -6,6 +6,7 @@ package engine import ( + "fmt" "os" "time" @@ -13,6 +14,7 @@ import ( "krino/internal/config" "krino/internal/extract" "krino/internal/ignore" + "krino/internal/plan" ) // Engine holds a loaded, compiled configuration: everything a front end @@ -84,6 +86,10 @@ func Load(mainFile string, names ...string) (*Engine, []*config.Diag) { errs = append(errs, cerrs...) continue } + if diag := checkCaptures(d.File, r, c); diag != nil { + errs = append(errs, diag) + continue + } dir.Rules = append(dir.Rules, &Rule{Name: r.Name, Conf: r, Settings: rs, Cond: c}) } dir.ContentVariants = contentVariants(dir.Rules) @@ -102,6 +108,38 @@ func Load(mainFile string, names ...string) (*Engine, []*config.Diag) { }, nil } +// checkCaptures validates a compiled rule's actions against the capture +// groups its own name tests can supply (spec 7.3): a rule using {N} needs a +// name test at all, and every name test in it needs at least N groups. It +// reports only the first offending action, so one config mistake yields one +// diagnostic. +func checkCaptures(file string, r *config.Rule, c *cond.Cond) *config.Diag { + groups := c.NameGroups() + for _, a := range r.Actions { + n, err := plan.MaxIndex(a.Arg) + if err != nil || n == 0 { + continue + } + if len(groups) == 0 { + return &config.Diag{File: file, Pos: a.Pos, Msg: fmt.Sprintf("rule %q: {%d} needs a name test to capture from", r.Name, n)} + } + for _, g := range groups { + if g < n { + return &config.Diag{File: file, Pos: a.Pos, Msg: fmt.Sprintf("rule %q: {%d} but a name test has only %s", r.Name, n, captureGroups(g))} + } + } + } + return nil +} + +// captureGroups renders a capture-group count with correct singular/plural. +func captureGroups(n int) string { + if n == 1 { + return "1 capture group" + } + return fmt.Sprintf("%d capture groups", n) +} + // dedupeNames returns names with every repeat after its first occurrence // removed, order preserved. func dedupeNames(names []string) []string { diff --git a/internal/engine/engine_test.go b/internal/engine/engine_test.go index 5c0536f..8dcfcb3 100644 --- a/internal/engine/engine_test.go +++ b/internal/engine/engine_test.go @@ -138,6 +138,55 @@ func TestCheck(t *testing.T) { } } +// TestLoadRejectsUnsuppliedCaptures: a rule using {N} must be able to get it +// from its own name tests (spec 7.3) — adapted from the brief to this +// package's actual sandbox/writeConfig/Load helpers (writeConfig takes a +// main file body and a dirs map; there is no separate engineLoad, Load is +// called directly). +func TestLoadRejectsUnsuppliedCaptures(t *testing.T) { + tests := []struct{ rule, want string }{ + {`(rule "a" (when (type pdf)) (move "Work/{1}"))`, + `rule "a": {1} needs a name test to capture from`}, + {`(rule "a" (when (name "inv-(\d+)")) (move "Work/{2}"))`, + `rule "a": {2} but a name test has only 1 capture group`}, + {`(rule "a" (when (or (name "x-(\d+)-(\d+)") (name "y-(\d+)"))) (rename "{2}"))`, + `rule "a": {2} but a name test has only 1 capture group`}, + } + for _, tt := range tests { + h := sandbox(t) + main := writeConfig(t, h, `(include "dl")`, map[string]string{"dl": `(path "/tmp") +` + tt.rule}) + _, errs := Load(main, "dl") + joined := "" + for _, d := range errs { + joined += d.Error() + "\n" + } + if len(errs) != 1 || !strings.Contains(joined, tt.want) { + t.Errorf("rule %s: errs %v, want %q", tt.rule, errs, tt.want) + } + } +} + +// TestLoadAcceptsSuppliedCaptures: a rule whose name test has enough groups +// for every {N} it uses loads clean. +func TestLoadAcceptsSuppliedCaptures(t *testing.T) { + for _, rule := range []string{ + `(rule "a" (when (name "inv-(\d+)-(\d+)")) (move "Work/{2}/{1}"))`, + // B1a: a name test reachable only under a (not ...) must not count + // toward this rule's own captures (B1), but it must also not make + // the rule itself invalid - the outer (name ...) alone already + // supplies the two groups {2} needs. + `(rule "a" (when (and (name "inv-(\d+)-(\d+)") (not (name "draft")))) (move "Work/{2}"))`, + } { + h := sandbox(t) + main := writeConfig(t, h, `(include "dl")`, map[string]string{"dl": `(path "/tmp") +` + rule}) + if _, errs := Load(main, "dl"); len(errs) != 0 { + t.Errorf("rule %s: unexpected diagnostics: %v", rule, errs) + } + } +} + // TestContentVariantsComputedAtLoad: B2 plumbing. Load computes each // directory's distinct (ignoreCase, fold) content-test variants from its // rules' resolved settings: a rule with no content test contributes diff --git a/internal/engine/facts.go b/internal/engine/facts.go index 60380f3..423bfd9 100644 --- a/internal/engine/facts.go +++ b/internal/engine/facts.go @@ -13,6 +13,7 @@ import ( "krino/internal/cond" "krino/internal/dup" "krino/internal/norm" + "krino/internal/plan" "krino/internal/scan" "krino/internal/xdg" ) @@ -166,7 +167,7 @@ func (f *facts) Duplicate(dirs []string) (string, bool, error) { root := f.run.d.Root resolved := make([]string, len(dirs)) for i, raw := range dirs { - resolved[i] = resolveDir(raw, root) + resolved[i] = plan.ResolveDir(raw, root) } sorted := append([]string(nil), resolved...) sort.Strings(sorted) @@ -183,16 +184,6 @@ func (f *facts) Duplicate(dirs []string) (string, bool, error) { return displayOriginal(orig, root), true, nil } -// resolveDir expands a leading ~ and joins a relative directory to root, -// cleaned. -func resolveDir(raw, root string) string { - p := xdg.Expand(raw) - if !filepath.IsAbs(p) { - p = filepath.Join(root, p) - } - return filepath.Clean(p) -} - // displayOriginal reports orig relative to root when it lies inside root, // else as an absolute path. func displayOriginal(orig, root string) string { diff --git a/internal/engine/match.go b/internal/engine/match.go index 0693bd5..e68a6e8 100644 --- a/internal/engine/match.go +++ b/internal/engine/match.go @@ -16,6 +16,7 @@ import ( "krino/internal/cond" "krino/internal/config" + "krino/internal/plan" "krino/internal/scan" "krino/internal/xdg" ) @@ -312,7 +313,12 @@ func isBusy(path string, suffixes []string) bool { // excludeDirs computes the directories Match and Explain never enter: each // rule's copy/move destination, the Trash, and the directory holding the -// main config file - each kept only when it lies strictly inside d's root. +// main config file - each kept only when it lies strictly inside d's root +// (C3: root itself is not "inside" it here - a rule cannot exclude the very +// directory being scanned. cmd/krino/render.go's relToRoot answers a +// different question, whether a destination is root or beneath it for +// display purposes, and there root does count as inside; the two are each +// correct for their own question, so do not "unify" them). // A destination with no template placeholder excludes exactly that // directory; a destination with a placeholder excludes only the static // part before its first "{", cut back to a full path component (its last @@ -348,7 +354,7 @@ func (e *Engine) excludeDirs(d *Dir) []string { prefix = "" } } - add(resolveDir(prefix, root)) + add(plan.ResolveDir(prefix, root)) } } add(filepath.Join(xdg.DataHome(), "Trash")) diff --git a/internal/engine/plan.go b/internal/engine/plan.go new file mode 100644 index 0000000..979bec1 --- /dev/null +++ b/internal/engine/plan.go @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package engine + +import ( + "context" + "time" + + "krino/internal/plan" +) + +// DirPlan is one directory's plan: the match Result it was built from (for +// the counts and warnings a front end renders), the chains plan.Build +// produced from its matched files, and Elapsed (D12), which - unlike +// Result.Elapsed, stopped inside Match before plan.Build ever runs - spans +// both Match and Build, so a front end reporting how long planning took is +// not undercounting it. It lives here, not in internal/plan, because +// internal/plan must not import internal/engine (engine already imports +// plan). +type DirPlan struct { + Dir *Dir + Chains []plan.Chain + Result *Result + Elapsed time.Duration +} + +// Plan matches d, then turns every matched file into a chain per spec +// §7.1. Unmatched files never reach plan.Build: they have no rules, hence +// no steps, and Result.Unmatched already says they were not matched. +// Chains keep Match's Rel order, since Matched is already sorted that way +// and Build's output lines up with its input position for position. claims +// is the run-scoped claim set (A3): the caller creates one *plan.Claims per +// run and passes the same instance to every directory's Plan call, so two +// directories claiming one destination resolve the collision instead of +// both silently landing on it. +func (e *Engine) Plan(ctx context.Context, d *Dir, claims *plan.Claims) (*DirPlan, error) { + started := time.Now() + r, err := e.Match(ctx, d) + if err != nil { + return nil, err + } + + inputs := make([]plan.Input, len(r.Matched)) + for i, fm := range r.Matched { + rules := make([]plan.RuleMatch, len(fm.Rules)) + for j, rm := range fm.Rules { + rules[j] = plan.RuleMatch{ + Name: rm.Rule.Name, + Actions: rm.Rule.Conf.Actions, + Settings: rm.Rule.Settings, + Captures: rm.Captures, + Reasons: rm.Reasons, + } + } + inputs[i] = plan.Input{File: fm.File, Rules: rules} + } + + chains := plan.Build(d.Root, inputs, e.Now(), plan.OS{}, claims) + return &DirPlan{Dir: d, Chains: chains, Result: r, Elapsed: time.Since(started)}, nil +} diff --git a/internal/engine/plan_test.go b/internal/engine/plan_test.go new file mode 100644 index 0000000..a169168 --- /dev/null +++ b/internal/engine/plan_test.go @@ -0,0 +1,146 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package engine + +import ( + "context" + "fmt" + "os" + "path/filepath" + "testing" + "time" + + "krino/internal/plan" +) + +func TestPlanBuildsChainsFromMatchedFiles(t *testing.T) { + h := sandbox(t) + os.Mkdir(filepath.Join(h, "dl"), 0o755) + old := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + for _, n := range []string{"z.pdf", "a.pdf", "notes.txt"} { + p := filepath.Join(h, "dl", n) + if err := os.WriteFile(p, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(p, old, old); err != nil { + t.Fatal(err) + } + } + main := writeConfig(t, h, `(include "dl")`, map[string]string{"dl": ` +(path "~/dl") +(min-age 0s) +(rule "pdfs" (when (type pdf)) (move "PDF")) +`}) + e, errs := Load(main, "dl") + if len(errs) > 0 { + t.Fatal(errs) + } + + dp, err := e.Plan(context.Background(), e.Dirs[0], plan.NewClaims()) + if err != nil { + t.Fatal(err) + } + if dp.Dir != e.Dirs[0] { + t.Errorf("Dir = %v, want %v", dp.Dir, e.Dirs[0]) + } + if dp.Result == nil || len(dp.Result.Matched) != 2 || len(dp.Result.Unmatched) != 1 { + t.Fatalf("Result = %+v", dp.Result) + } + + // Chains keep Match's Rel order: "a.pdf" before "z.pdf", regardless of + // walk or map-iteration order. + if len(dp.Chains) != 2 { + t.Fatalf("got %d chains, want 2 (unmatched notes.txt must not appear)", len(dp.Chains)) + } + if dp.Chains[0].File.Rel != "a.pdf" || dp.Chains[1].File.Rel != "z.pdf" { + t.Fatalf("chains not in Rel order: %s, %s", dp.Chains[0].File.Rel, dp.Chains[1].File.Rel) + } + + c := dp.Chains[0] + if len(c.Steps) != 1 { + t.Fatalf("a.pdf steps = %+v", c.Steps) + } + s := c.Steps[0] + want := filepath.Join(h, "dl", "PDF", "a.pdf") + if s.Kind != plan.Move || s.Rule != "pdfs" || s.Dst != want || s.Skip != "" { + t.Errorf("step = %+v, want Move to %s", s, want) + } +} + +// TestPlanSharesClaimsAcrossDirectories: A3 - plan.Claims is created once +// by the caller and threaded through every Engine.Plan call of a run, so +// two configured directories cannot plan the same final name. Two +// directories each hold a file named "x.txt" and each carry a rule moving +// it to one shared destination outside both roots; calling Plan for both +// with the *same* claims must resolve the second directory's step onto +// "x_1.txt" rather than let it collide on "x.txt" too. +func TestPlanSharesClaimsAcrossDirectories(t *testing.T) { + h := sandbox(t) + shared := filepath.Join(h, "elsewhere") + for _, d := range []string{"d1", "d2"} { + if err := os.Mkdir(filepath.Join(h, d), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(h, d, "x.txt"), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + } + dirConf := func(path string) string { + return fmt.Sprintf(`(path %q) +(min-age 0s) +(rule "r" (when (type txt)) (move %q)) +`, path, shared) + } + main := writeConfig(t, h, `(include "d1" "d2")`, map[string]string{ + "d1": dirConf("~/d1"), + "d2": dirConf("~/d2"), + }) + e, errs := Load(main, "d1", "d2") + if len(errs) > 0 { + t.Fatal(errs) + } + + claims := plan.NewClaims() + dp1, err := e.Plan(context.Background(), e.Dirs[0], claims) + if err != nil { + t.Fatal(err) + } + dp2, err := e.Plan(context.Background(), e.Dirs[1], claims) + if err != nil { + t.Fatal(err) + } + + if len(dp1.Chains) != 1 || len(dp1.Chains[0].Steps) != 1 { + t.Fatalf("d1 chains = %+v", dp1.Chains) + } + if len(dp2.Chains) != 1 || len(dp2.Chains[0].Steps) != 1 { + t.Fatalf("d2 chains = %+v", dp2.Chains) + } + got1 := dp1.Chains[0].Steps[0].Dst + got2 := dp2.Chains[0].Steps[0].Dst + want1 := filepath.Join(shared, "x.txt") + want2 := filepath.Join(shared, "x_1.txt") + if got1 != want1 { + t.Errorf("d1 dst = %s, want %s", got1, want1) + } + if got2 != want2 { + t.Errorf("d2 dst = %s, want %s (claims must be shared with d1's plan)", got2, want2) + } +} + +func TestPlanReturnsMatchError(t *testing.T) { + h := sandbox(t) + main := writeConfig(t, h, `(include "dl")`, map[string]string{"dl": ` +(path "~/dl") +(rule "r" (when (type pdf)) (move "PDF")) +`}) + e, errs := Load(main, "dl") + if len(errs) > 0 { + t.Fatal(errs) + } + // dl's root ~/dl does not exist: scan.Walk fails, and Plan must + // propagate that error rather than paper over it with an empty plan. + if _, err := e.Plan(context.Background(), e.Dirs[0], plan.NewClaims()); err == nil { + t.Error("want an error when the root does not exist, got nil") + } +} diff --git a/internal/plan/chain.go b/internal/plan/chain.go new file mode 100644 index 0000000..bfa2484 --- /dev/null +++ b/internal/plan/chain.go @@ -0,0 +1,222 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package plan + +import ( + "fmt" + "path/filepath" + "sort" + "strings" + "time" + + "krino/internal/config" + "krino/internal/scan" + "krino/internal/xdg" +) + +// Claims is the set of destination paths already spoken for, shared across +// every directory planned in one run (A3): two directories competing for +// one destination must resolve the collision at planning time, which needs +// one Claims threaded through every Build call of that run, not a fresh one +// per call. +type Claims struct { + taken claimed +} + +// NewClaims returns an empty Claims, ready to pass to Build. +func NewClaims() *Claims { + return &Claims{taken: claimed{}} +} + +// Input is one file and the rules that matched it, in match order. +type Input struct { + File scan.File + Rules []RuleMatch +} + +// Build turns each file's matching rules into a chain. root is the +// directory's absolute root; now is the start of the run; d is consulted to +// resolve destination conflicts (§7.4). claims is the run-scoped claim set +// (A3): pass the same *Claims to every Build call of one run (every +// directory included) so two directories claiming one destination resolve +// the collision instead of both silently landing on it. claims must not be +// nil - a caller with nothing to share yet still calls NewClaims() itself, +// so an accidentally-unshared claim set can never happen by simply +// forgetting the argument. Files whose chain has no steps are returned with +// an empty Steps slice, so the caller can tell "matched a rule that does +// nothing" from "not matched". +// +// Conflicts are resolved as each step is created, and a claimed destination +// is shared across every file's chain, not just its own: two files +// competing for one name must resolve the same way on every run, so +// resolution proceeds in File.Rel order regardless of the order in which +// in is given. The returned slice still lines up with in, position for +// position. +func Build(root string, in []Input, now time.Time, d Disk, claims *Claims) []Chain { + if claims == nil { + panic("plan: Build requires a non-nil Claims (see NewClaims)") + } + order := make([]int, len(in)) + for i := range order { + order[i] = i + } + sort.SliceStable(order, func(i, j int) bool { + return in[order[i]].File.Rel < in[order[j]].File.Rel + }) + + chains := make([]Chain, len(in)) + for _, i := range order { + chains[i] = buildOne(root, in[i], now, d, claims.taken) + } + return chains +} + +// buildOne builds the chain for a single file. claim is shared with every +// other file processed by the same Build call. +func buildOne(root string, in Input, now time.Time, d Disk, claim claimed) Chain { + c := Chain{File: in.File} + cur := in.File.Path + + var deletedBy string + moves := 0 + warnedMove := false + + for _, rule := range in.Rules { + reason := strings.Join(rule.Reasons, ", ") + for _, a := range rule.Actions { + kind := stepKind(a.Kind) + + if deletedBy != "" { + c.Steps = append(c.Steps, Step{ + Kind: kind, + Rule: rule.Name, + Src: cur, + Reason: reason, + Skip: "deleted by rule " + deletedBy, + Conflict: rule.Settings.OnConflict, + }) + continue + } + + facts := Facts{ + Name: filepath.Base(cur), + Captures: rule.Captures, + ModTime: in.File.ModTime, + Now: now, + } + + step := Step{Kind: kind, Rule: rule.Name, Src: cur, Reason: reason, Conflict: rule.Settings.OnConflict} + + switch a.Kind { + case config.Copy, config.Move: + // D9: a placeholder failure (below) leaves step.Dst empty - + // there was never a destination to compute at all - while a + // conflict-policy skip (via resolveConflict, right after) + // always sets step.Dst: even when the step will not run, + // its would-be destination is a real, already-resolved + // path worth showing. + dest, err := expandDir(a.Arg, facts, root) + if err != nil { + step.Skip = err.Error() + break + } + dst := filepath.Join(dest, filepath.Base(cur)) + resolved, skip, displaces := resolveConflict(kind, rule.Settings.OnConflict, cur, dst, d, claim) + step.Dst = resolved + step.Skip = skip + step.Displaces = displaces + if skip == "" { + // C4: the path cur is about to be vacated from (on a + // move) enters neither claim nor any "freed" set, so + // Disk.Exists still reports it occupied for the rest of + // this plan and a later file wanting that exact name + // gets a gratuitous _1. This errs safe - it never lets + // a name be claimed before its file has actually + // vacated it - and stays; do not "fix" it by weakening + // the disk check. + claim[resolved] = true + if a.Kind == config.Move { + cur = resolved + moves++ + if moves > 1 && !warnedMove { + c.Warnings = append(c.Warnings, "moved more than once; a (stop) is probably missing") + warnedMove = true + } + } + } + + case config.Rename: + name, err := Expand(a.Arg, facts) + if err != nil { + step.Skip = err.Error() + break + } + if strings.ContainsRune(name, '/') { + step.Skip = `rename produced a name containing "/"` + break + } + dst := filepath.Join(filepath.Dir(cur), name) + resolved, skip, displaces := resolveConflict(kind, rule.Settings.OnConflict, cur, dst, d, claim) + step.Dst = resolved + step.Skip = skip + step.Displaces = displaces + if skip == "" { + cur = resolved + claim[resolved] = true + } + + case config.Delete, config.DeletePermanent: + deletedBy = rule.Name + } + + c.Steps = append(c.Steps, step) + } + } + + return c +} + +// stepKind maps a config.ActionKind onto its plan.Kind, exhaustively +// (D4): an unrecognised ActionKind panics rather than silently reading as +// config.Delete, matching config.ActionKind.String()'s own exhaustive style +// with an explicit fallback. +func stepKind(k config.ActionKind) Kind { + switch k { + case config.Copy: + return Copy + case config.Move: + return Move + case config.Rename: + return Rename + case config.DeletePermanent: + return DeletePermanent + case config.Delete: + return Trash + } + panic(fmt.Sprintf("plan: unknown config.ActionKind %d", int(k))) +} + +// expandDir expands raw (a DEST argument) against facts, then resolves it +// the same way the engine resolves an extra directory: ~ expands, a +// relative path joins root, and the result is cleaned. +func expandDir(raw string, facts Facts, root string) (string, error) { + expanded, err := Expand(raw, facts) + if err != nil { + return "", err + } + return ResolveDir(expanded, root), nil +} + +// ResolveDir expands a leading ~ and joins a relative directory to root, +// cleaned. C1: this is the one place that decides where a rule's +// destination resolves to; internal/engine calls it too (its own directory +// walk needs to agree on the same paths), rather than keeping a second, +// separately-maintained copy - plan is the lower layer (engine imports +// plan, so plan must never import engine), so the decision belongs here. +func ResolveDir(raw, root string) string { + p := xdg.Expand(raw) + if !filepath.IsAbs(p) { + p = filepath.Join(root, p) + } + return filepath.Clean(p) +} diff --git a/internal/plan/chain_test.go b/internal/plan/chain_test.go new file mode 100644 index 0000000..b6e3dfe --- /dev/null +++ b/internal/plan/chain_test.go @@ -0,0 +1,143 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package plan + +import ( + "path/filepath" + "testing" + "time" + + "krino/internal/config" + "krino/internal/scan" +) + +func file(root, rel string) scan.File { + return scan.File{ + Path: filepath.Join(root, rel), + Rel: rel, + Name: filepath.Base(rel), + Size: 10, + ModTime: time.Date(2026, 8, 15, 12, 0, 0, 0, time.UTC), + } +} + +func act(k config.ActionKind, arg string) config.Action { return config.Action{Kind: k, Arg: arg} } + +func TestBuildChain(t *testing.T) { + root := "/r" + now := time.Date(2026, 9, 12, 0, 0, 0, 0, time.UTC) + in := []Input{{ + File: file(root, "inv1.pdf"), + Rules: []RuleMatch{ + {Name: "backup", Actions: []config.Action{act(config.Copy, "/backup/{mtime:%Y}")}}, + {Name: "acme", Actions: []config.Action{ + act(config.Rename, "{mtime:%Y-%m-%d}_{name}"), + act(config.Move, "Work/Acme/{mtime:%Y}"), + }}, + }, + }} + got := Build(root, in, now, NoDisk{}, NewClaims()) + if len(got) != 1 || len(got[0].Steps) != 3 { + t.Fatalf("got %d chains / %d steps", len(got), len(got[0].Steps)) + } + want := []Step{ + {Kind: Copy, Rule: "backup", Src: "/r/inv1.pdf", Dst: "/backup/2026/inv1.pdf"}, + {Kind: Rename, Rule: "acme", Src: "/r/inv1.pdf", Dst: "/r/2026-08-15_inv1.pdf"}, + {Kind: Move, Rule: "acme", Src: "/r/2026-08-15_inv1.pdf", Dst: "/r/Work/Acme/2026/2026-08-15_inv1.pdf"}, + } + for i, w := range want { + g := got[0].Steps[i] + if g.Kind != w.Kind || g.Rule != w.Rule || g.Src != w.Src || g.Dst != w.Dst || g.Skip != "" { + t.Errorf("step %d = %+v; want %+v", i, g, w) + } + } + if len(got[0].Warnings) != 0 { + t.Errorf("unexpected warnings: %v", got[0].Warnings) + } +} + +func TestBuildDeleteEndsChain(t *testing.T) { + in := []Input{{ + File: file("/r", "old.iso"), + Rules: []RuleMatch{ + {Name: "dups", Actions: []config.Action{act(config.Delete, "")}}, + {Name: "archive", Actions: []config.Action{act(config.Move, "Archive")}}, + }, + }} + steps := Build("/r", in, time.Now(), NoDisk{}, NewClaims())[0].Steps + if len(steps) != 2 || steps[0].Kind != Trash || steps[0].Dst != "" { + t.Fatalf("steps = %+v", steps) + } + if steps[1].Skip != "deleted by rule dups" { + t.Errorf("step after delete: Skip = %q", steps[1].Skip) + } +} + +func TestBuildWarnsOnTwoMoves(t *testing.T) { + in := []Input{{ + File: file("/r", "x.pdf"), + Rules: []RuleMatch{ + {Name: "a", Actions: []config.Action{act(config.Move, "A")}}, + {Name: "b", Actions: []config.Action{act(config.Move, "B")}}, + }, + }} + c := Build("/r", in, time.Now(), NoDisk{}, NewClaims())[0] + if len(c.Warnings) != 1 || c.Warnings[0] != "moved more than once; a (stop) is probably missing" { + t.Errorf("warnings = %v", c.Warnings) + } + if c.Steps[1].Src != "/r/A/x.pdf" { + t.Errorf("second move reads from %q, want the path after the first move", c.Steps[1].Src) + } +} + +func TestBuildBadPlaceholderSkipsOneStep(t *testing.T) { + in := []Input{{ + File: file("/r", "x.pdf"), + Rules: []RuleMatch{{Name: "a", Actions: []config.Action{ + act(config.Move, "Work/{1}"), + act(config.Rename, "ok-{name}"), + }}}, + }} + c := Build("/r", in, time.Now(), NoDisk{}, NewClaims())[0] + if c.Steps[0].Skip == "" { + t.Errorf("step with {1} and no captures should be skipped: %+v", c.Steps[0]) + } + if c.Steps[1].Skip != "" || c.Steps[1].Dst != "/r/ok-x.pdf" { + t.Errorf("chain should continue from the unchanged path: %+v", c.Steps[1]) + } +} + +func TestBuildRenameWithSlash(t *testing.T) { + in := []Input{{ + File: file("/r", "x.pdf"), + Rules: []RuleMatch{{Name: "a", Actions: []config.Action{act(config.Rename, "sub/{name}")}}}, + }} + c := Build("/r", in, time.Now(), NoDisk{}, NewClaims())[0] + if c.Steps[0].Skip != `rename produced a name containing "/"` { + t.Errorf("Skip = %q", c.Steps[0].Skip) + } +} + +// TestBuildKeepsSteplessChains is D6: Build returns one Chain per Input even +// when a file's rules contribute no actions, so a caller can tell "matched a +// rule that does nothing" (an exclusion) from "not matched at all". Plan 3's +// "to act on" count and the JSON document's empty steps array both rest on +// this. +func TestBuildKeepsSteplessChains(t *testing.T) { + in := []Input{ + {File: file("/r", "excluded.txt"), Rules: []RuleMatch{{Name: "only-stop"}}}, + {File: file("/r", "untouched.txt")}, + } + chains := Build("/r", in, time.Now(), NoDisk{}, NewClaims()) + if len(chains) != 2 { + t.Fatalf("got %d chains, want one per Input", len(chains)) + } + for i, c := range chains { + if len(c.Steps) != 0 { + t.Errorf("chain %d: got %d steps, want none", i, len(c.Steps)) + } + if c.File.Rel != in[i].File.Rel { + t.Errorf("chain %d: File.Rel = %q, want %q (positional alignment)", i, c.File.Rel, in[i].File.Rel) + } + } +} diff --git a/internal/plan/conflict.go b/internal/plan/conflict.go new file mode 100644 index 0000000..8747669 --- /dev/null +++ b/internal/plan/conflict.go @@ -0,0 +1,134 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package plan + +import ( + "fmt" + "os" + "path/filepath" + + "krino/internal/config" + "krino/internal/dup" +) + +// Disk is what Build needs from the filesystem to resolve conflicts, so +// tests can supply a stub and Build stays pure otherwise. +type Disk interface { + Exists(path string) bool + SameContent(a, b string) (bool, error) +} + +// OS is the real filesystem. +type OS struct{} + +// Exists reports whether path names an existing file or directory, without +// following a symlink at path itself: a dangling or otherwise unwanted +// symlink still counts as "something is there". +func (OS) Exists(path string) bool { + _, err := os.Lstat(path) + return err == nil +} + +// SameContent delegates to internal/dup, the one place content identity is +// decided. +func (OS) SameContent(a, b string) (bool, error) { + return dup.SameContent(a, b) +} + +// NoDisk is the empty filesystem: nothing exists, and nothing is ever the +// same content. For tests that are not about conflicts. +type NoDisk struct{} + +func (NoDisk) Exists(string) bool { return false } +func (NoDisk) SameContent(string, string) (bool, error) { return false, nil } + +// claimed is the set of destination paths already spoken for by an earlier +// step of this plan. +type claimed map[string]bool + +// resolveConflict decides what a step whose destination is contested does, +// per spec §7.4. src is the step's source (its current path, before this +// step runs); dst is the target the action computed. It returns the +// resolved destination (possibly unchanged), a Skip reason (non-empty when +// the step must not run) and Displaces (non-empty only for overwrite of a +// file that exists on disk). +func resolveConflict(kind Kind, policy config.Conflict, src, dst string, d Disk, c claimed) (resolved, skip, displaces string) { + // A1/A2: the file is already where this step would put it, so its own + // existence must not read as a conflict with itself. Without this guard a + // move or rename plans a rename to stem_1 and every later run adds + // another generation; under (on-conflict overwrite) the step records the + // file as its own Displaces, which plan 4 would trash before moving from + // a path that no longer exists. Checked before the policy switch, so + // overwrite never reaches its own branch. + if dst == src { + return dst, "already there", "" + } + + onDisk := d.Exists(dst) + + if kind == Copy && onDisk { + // A SameContent error (the source vanished, a permission problem, + // ...) is treated the same as "different content": the step falls + // through to the ordinary conflict policy below instead of failing + // outright. A wrong "different" verdict costs at worst an + // unnecessary suffixed copy, never data loss, so resolving the + // conflict anyway is an acceptable trade-off here (D8) - a caller + // that wants the failure itself visible would need it surfaced as + // a chain warning instead. + if same, err := d.SameContent(src, dst); err == nil && same { + return dst, "already there", "" + } + } + + if !onDisk && !c[dst] { + return dst, "", "" + } + + switch policy { + case config.ConflictSkip: + return dst, "target exists", "" + case config.ConflictOverwrite: + if onDisk && !c[dst] { + // The existing file is trashed first (plan 4). Only the first + // step to reach this path may displace it: once another step + // in this same plan has already claimed dst, that path will + // hold that step's own output by the time this one runs, so + // displacing it again would destroy it. + return dst, "", dst + } + // Either claimed in-plan only (nothing on disk to displace — an + // in-plan claim is never displaced), or on disk but already + // claimed by an earlier step of this plan (displacing it again + // would destroy that step's output): either way the two chains + // cannot share one destination, so fall back to a free name, + // exactly as suffix would. A step that takes a free name + // displaces nothing. + resolved, skip := suffixed(dst, d, c) + return resolved, skip, "" + default: // config.ConflictSuffix + resolved, skip := suffixed(dst, d, c) + return resolved, skip, "" + } +} + +// maxSuffixAttempts bounds suffixed(): it is unbounded by design and +// terminates on a real filesystem, but C2 - without a cap, a Disk that +// always reports existence (or a directory A1 had been filling before its +// fix) turns planning quadratic instead of failing fast. +const maxSuffixAttempts = 10000 + +// suffixed finds the first stem_N.ext (N starting at 1) that is free: +// neither on disk nor already claimed by an earlier step in this plan. It +// gives up after maxSuffixAttempts, returning a Skip reason and no path +// (C2). +func suffixed(dst string, d Disk, c claimed) (resolved, skip string) { + dir, base := filepath.Split(dst) + stem, ext := splitExt(base) + for n := 1; n <= maxSuffixAttempts; n++ { + candidate := filepath.Join(dir, fmt.Sprintf("%s_%d%s", stem, n, ext)) + if !d.Exists(candidate) && !c[candidate] { + return candidate, "" + } + } + return "", "too many conflicting names" +} diff --git a/internal/plan/conflict_test.go b/internal/plan/conflict_test.go new file mode 100644 index 0000000..bdc37af --- /dev/null +++ b/internal/plan/conflict_test.go @@ -0,0 +1,224 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package plan + +import ( + "os" + "path/filepath" + "testing" + "time" + + "krino/internal/config" +) + +// fakeDisk reports the paths it was given as existing, and equality of +// content by exact string match on a separate map. +type fakeDisk struct { + exists map[string]bool + same map[[2]string]bool +} + +func (f fakeDisk) Exists(p string) bool { return f.exists[p] } +func (f fakeDisk) SameContent(a, b string) (bool, error) { + return f.same[[2]string{a, b}], nil +} + +// TestConflictAlreadyThereMove is A1: a move whose destination resolves to +// the directory the file is already sitting in must be a no-op, not a +// rename - probe: "(rule "byyear" (when (type txt)) (move "{mtime:%Y}"))" +// over "dl/2026/a.txt" planned "a.txt -> a_1.txt" on run 1 and, on run 2, +// both "a.txt -> a_2.txt" and "a_1.txt -> a_1_1.txt": every run renamed the +// whole destination tree and added a generation, because the file's own +// existence at dst read as a conflict with itself. +func TestConflictAlreadyThereMove(t *testing.T) { + d := fakeDisk{exists: map[string]bool{"/r/2026/a.txt": true}} + in := []Input{{File: file("/r", "2026/a.txt"), Rules: []RuleMatch{ + {Name: "byyear", Actions: []config.Action{act(config.Move, "2026")}}}}} + s := Build("/r", in, time.Now(), d, NewClaims())[0].Steps[0] + if s.Dst != "/r/2026/a.txt" || s.Skip != "already there" || s.Displaces != "" { + t.Errorf("step = %+v; want a no-op at the file's own path", s) + } +} + +// TestConflictAlreadyThereRename is A1's rename counterpart: +// (rename "{name}") over a file already named that must also be a no-op. +func TestConflictAlreadyThereRename(t *testing.T) { + d := fakeDisk{exists: map[string]bool{"/r/2026/a.txt": true}} + in := []Input{{File: file("/r", "2026/a.txt"), Rules: []RuleMatch{ + {Name: "byyear", Actions: []config.Action{act(config.Rename, "{name}")}}}}} + s := Build("/r", in, time.Now(), d, NewClaims())[0].Steps[0] + if s.Dst != "/r/2026/a.txt" || s.Skip != "already there" || s.Displaces != "" { + t.Errorf("step = %+v; want a no-op at the file's own path", s) + } +} + +// TestConflictAlreadyThereOverwriteNoDisplace is A2: the same already-there +// case under (on-conflict overwrite) must not record the file as its own +// Displaces - spec §7.4's overwrite moves the existing target to Trash +// first, then proceeds; applied literally here that trashes the user's file +// and then moves from a path that no longer exists, so the file survives +// only in Trash. A1's guard (dst == src, checked before the policy switch) +// fixes this too, since it runs before overwrite's own branch is ever +// reached. +func TestConflictAlreadyThereOverwriteNoDisplace(t *testing.T) { + d := fakeDisk{exists: map[string]bool{"/r/2026/a.txt": true}} + over := config.ConflictOverwrite + in := []Input{{File: file("/r", "2026/a.txt"), Rules: []RuleMatch{ + {Name: "byyear", Settings: config.Resolved{OnConflict: over}, Actions: []config.Action{act(config.Move, "2026")}}}}} + s := Build("/r", in, time.Now(), d, NewClaims())[0].Steps[0] + if s.Skip != "already there" || s.Displaces != "" { + t.Errorf("step = %+v; want Skip \"already there\" and empty Displaces", s) + } +} + +func TestConflictSuffix(t *testing.T) { + d := fakeDisk{exists: map[string]bool{"/r/Work/x.pdf": true, "/r/Work/x_1.pdf": true}} + in := []Input{{File: file("/r", "x.pdf"), Rules: []RuleMatch{ + {Name: "a", Actions: []config.Action{act(config.Move, "Work")}}}}} + s := Build("/r", in, time.Now(), d, NewClaims())[0].Steps[0] + if s.Dst != "/r/Work/x_2.pdf" || s.Skip != "" { + t.Errorf("step = %+v; want Dst /r/Work/x_2.pdf", s) + } +} + +func TestConflictSkipKeepsCurrentPath(t *testing.T) { + d := fakeDisk{exists: map[string]bool{"/r/Work/x.pdf": true}} + skip := config.ConflictSkip + in := []Input{{File: file("/r", "x.pdf"), Rules: []RuleMatch{ + {Name: "a", Settings: config.Resolved{OnConflict: skip}, Actions: []config.Action{ + act(config.Move, "Work"), + act(config.Rename, "later-{name}"), + }}}}} + steps := Build("/r", in, time.Now(), d, NewClaims())[0].Steps + if steps[0].Skip != "target exists" { + t.Errorf("skip step = %+v", steps[0]) + } + if steps[1].Src != "/r/x.pdf" || steps[1].Dst != "/r/later-x.pdf" { + t.Errorf("chain must continue from the unchanged path: %+v", steps[1]) + } +} + +func TestConflictOverwriteRecordsDisplaced(t *testing.T) { + d := fakeDisk{exists: map[string]bool{"/r/Work/x.pdf": true}} + over := config.ConflictOverwrite + in := []Input{{File: file("/r", "x.pdf"), Rules: []RuleMatch{ + {Name: "a", Settings: config.Resolved{OnConflict: over}, Actions: []config.Action{act(config.Move, "Work")}}}}} + s := Build("/r", in, time.Now(), d, NewClaims())[0].Steps[0] + if s.Dst != "/r/Work/x.pdf" || s.Displaces != "/r/Work/x.pdf" { + t.Errorf("step = %+v", s) + } +} + +// TestConflictOverwriteTwoFilesOnDisk: two files collide on one path that +// already exists on disk, both under overwrite. Only the first may displace +// the pre-existing file; the second must take a free name and displace +// nothing, or applying both later would trash the first file's own output. +func TestConflictOverwriteTwoFilesOnDisk(t *testing.T) { + d := fakeDisk{exists: map[string]bool{"/r/Work/x.pdf": true}} + over := config.ConflictOverwrite + in := []Input{ + {File: file("/r", "a/x.pdf"), Rules: []RuleMatch{ + {Name: "r", Settings: config.Resolved{OnConflict: over}, Actions: []config.Action{act(config.Move, "Work")}}}}, + {File: file("/r", "b/x.pdf"), Rules: []RuleMatch{ + {Name: "r", Settings: config.Resolved{OnConflict: over}, Actions: []config.Action{act(config.Move, "Work")}}}}, + } + chains := Build("/r", in, time.Now(), d, NewClaims()) + first, second := chains[0].Steps[0], chains[1].Steps[0] + if first.Dst != "/r/Work/x.pdf" || first.Displaces != "/r/Work/x.pdf" { + t.Errorf("first step = %+v; want Dst and Displaces /r/Work/x.pdf", first) + } + if second.Dst != "/r/Work/x_1.pdf" || second.Displaces != "" { + t.Errorf("second step = %+v; want Dst /r/Work/x_1.pdf and empty Displaces", second) + } +} + +// TestConflictOverwriteTwoFilesClaimedOnly: same collision, but nothing is +// on disk — the two files claim the same name only in-plan. Neither may +// displace (an in-plan claim is never displaced); the second must fall back +// to a free name with no Displaces recorded. +func TestConflictOverwriteTwoFilesClaimedOnly(t *testing.T) { + over := config.ConflictOverwrite + in := []Input{ + {File: file("/r", "a/x.pdf"), Rules: []RuleMatch{ + {Name: "r", Settings: config.Resolved{OnConflict: over}, Actions: []config.Action{act(config.Move, "Work")}}}}, + {File: file("/r", "b/x.pdf"), Rules: []RuleMatch{ + {Name: "r", Settings: config.Resolved{OnConflict: over}, Actions: []config.Action{act(config.Move, "Work")}}}}, + } + chains := Build("/r", in, time.Now(), NoDisk{}, NewClaims()) + first, second := chains[0].Steps[0], chains[1].Steps[0] + if first.Dst != "/r/Work/x.pdf" || first.Displaces != "" { + t.Errorf("first step = %+v; want Dst /r/Work/x.pdf and empty Displaces", first) + } + if second.Dst != "/r/Work/x_1.pdf" || second.Displaces != "" { + t.Errorf("second step = %+v; want Dst /r/Work/x_1.pdf and empty Displaces", second) + } +} + +func TestCopyAlreadyThere(t *testing.T) { + d := fakeDisk{ + exists: map[string]bool{"/backup/x.pdf": true}, + same: map[[2]string]bool{{"/r/x.pdf", "/backup/x.pdf"}: true}, + } + in := []Input{{File: file("/r", "x.pdf"), Rules: []RuleMatch{ + {Name: "b", Actions: []config.Action{act(config.Copy, "/backup")}}}}} + s := Build("/r", in, time.Now(), d, NewClaims())[0].Steps[0] + if s.Skip != "already there" { + t.Errorf("step = %+v; want Skip \"already there\"", s) + } +} + +func TestTwoFilesOneTarget(t *testing.T) { + in := []Input{ + {File: file("/r", "a/x.pdf"), Rules: []RuleMatch{ + {Name: "r", Actions: []config.Action{act(config.Move, "Work")}}}}, + {File: file("/r", "b/x.pdf"), Rules: []RuleMatch{ + {Name: "r", Actions: []config.Action{act(config.Move, "Work")}}}}, + } + chains := Build("/r", in, time.Now(), NoDisk{}, NewClaims()) + if chains[0].Steps[0].Dst != "/r/Work/x.pdf" || chains[1].Steps[0].Dst != "/r/Work/x_1.pdf" { + t.Errorf("in-plan collision: %q and %q", chains[0].Steps[0].Dst, chains[1].Steps[0].Dst) + } +} + +func TestSameContentReal(t *testing.T) { + dir := t.TempDir() + a := filepath.Join(dir, "a") + b := filepath.Join(dir, "b") + c := filepath.Join(dir, "c") + if err := os.WriteFile(a, []byte("same bytes"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(b, []byte("same bytes"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(c, []byte("other bytes"), 0o644); err != nil { + t.Fatal(err) + } + if ok, err := (OS{}).SameContent(a, b); err != nil || !ok { + t.Errorf("identical files: %v %v", ok, err) + } + if ok, _ := (OS{}).SameContent(a, c); ok { + t.Error("different files reported identical") + } +} + +// stubAlwaysExists is C2's stub Disk: every path reports as existing (and +// nothing is ever the same content), so suffixed() can never find a free +// name and must give up instead of spinning forever. +type stubAlwaysExists struct{} + +func (stubAlwaysExists) Exists(string) bool { return true } +func (stubAlwaysExists) SameContent(string, string) (bool, error) { return false, nil } + +// TestSuffixedCapsAttempts is C2: suffixed() gives up after +// maxSuffixAttempts, reporting Skip "too many conflicting names" and no +// Dst, rather than looping forever against a Disk that never reports a free +// name. +func TestSuffixedCapsAttempts(t *testing.T) { + in := []Input{{File: file("/r", "x.pdf"), Rules: []RuleMatch{ + {Name: "a", Actions: []config.Action{act(config.Move, "Work")}}}}} + s := Build("/r", in, time.Now(), stubAlwaysExists{}, NewClaims())[0].Steps[0] + if s.Skip != "too many conflicting names" || s.Dst != "" { + t.Errorf("step = %+v; want Skip \"too many conflicting names\" and empty Dst", s) + } +} diff --git a/internal/plan/index.go b/internal/plan/index.go new file mode 100644 index 0000000..abe073f --- /dev/null +++ b/internal/plan/index.go @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package plan + +import ( + "fmt" + "strconv" + "strings" +) + +// MaxIndex returns the highest {N} used in s, 0 when none. It reports the +// same errors Expand does for a malformed placeholder: an unclosed +// placeholder, or {0} (capture groups are numbered from 1). It shares +// Expand's scanner shape: "{{" and "}}" each emit one literal brace, and any +// other "{" opens a placeholder that runs to the next "}". +func MaxIndex(s string) (int, error) { + max := 0 + for i := 0; i < len(s); { + switch { + case s[i] == '{' && i+1 < len(s) && s[i+1] == '{': + i += 2 + case s[i] == '}' && i+1 < len(s) && s[i+1] == '}': + i += 2 + case s[i] == '{': + end := strings.IndexByte(s[i+1:], '}') + if end < 0 { + return 0, fmt.Errorf("unclosed placeholder") + } + body := s[i+1 : i+1+end] + if n, err := strconv.Atoi(body); err == nil { + if n == 0 { + return 0, fmt.Errorf("capture groups are numbered from 1") + } + if n >= 1 && n <= 9 && n > max { + max = n + } + } + i += end + 2 + default: + i++ + } + } + return max, nil +} diff --git a/internal/plan/index_test.go b/internal/plan/index_test.go new file mode 100644 index 0000000..b157f56 --- /dev/null +++ b/internal/plan/index_test.go @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package plan + +import ( + "strings" + "testing" +) + +func TestMaxIndex(t *testing.T) { + for in, want := range map[string]int{ + "Work/Acme": 0, + "{name}": 0, + "{1}": 1, + "Work/{2}/{1}": 2, + "{stem}-{9}{ext}": 9, + "{{1}}": 0, + "{mtime:%Y}/{3}-{1}": 3, + } { + got, err := MaxIndex(in) + if err != nil || got != want { + t.Errorf("MaxIndex(%q) = %d, %v; want %d", in, got, err, want) + } + } + if _, err := MaxIndex("{name"); err == nil || !strings.Contains(err.Error(), "unclosed placeholder") { + t.Errorf("unclosed: %v", err) + } +} diff --git a/internal/plan/json.go b/internal/plan/json.go new file mode 100644 index 0000000..0b4f1bc --- /dev/null +++ b/internal/plan/json.go @@ -0,0 +1,97 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package plan + +import "time" + +// jsonNote is carried in every JSON document, warning readers that the +// shape is not yet stable. +const jsonNote = "the shape of this document is unstable before krino 1.0" + +// JSON is the --json document. The shape is unstable before 1.0 and says +// so in its own "note" field. +type JSON struct { + Version int `json:"version"` + Note string `json:"note"` + Dirs []JSONDir `json:"dirs"` +} + +// 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"` +} + +// JSONFile is one file's chain. +type JSONFile struct { + Rel string `json:"rel"` + Size int64 `json:"size"` + ModTime time.Time `json:"mtime"` + Steps []JSONStep `json:"steps"` + Warnings []string `json:"warnings,omitempty"` +} + +// JSONStep is one step of a chain. D14: Reason is carried because the text +// plan already shows it and a machine reader should be able to see why a +// file matched too; Conflict (the rule's on-conflict policy) is deliberately +// not: it is a config detail, and its outcome is already visible through +// dst, displaces and skip. +type JSONStep struct { + Action string `json:"action"` + Rule string `json:"rule"` + Src string `json:"src"` + Dst string `json:"dst,omitempty"` + Displaces string `json:"displaces,omitempty"` + Reason string `json:"reason,omitempty"` + Skip string `json:"skip,omitempty"` +} + +// actionNames maps a Kind onto its JSON action name. This is its own +// mapping, independent of Kind.String(): the display form renders "DELETE +// permanently", which must never reach a machine reader. These names match +// the log's action names in spec §9, so a later `krino log` and a --json +// plan can be grepped together. +var actionNames = map[Kind]string{ + Copy: "copy", + Move: "move", + Rename: "rename", + Trash: "trash", + DeletePermanent: "delete", +} + +// NewJSON builds the top-level --json document over dirs. Version and Note +// are set here, in the one place jsonNote's wording already lives: it is +// unexported, so a struct literal built outside this package would +// silently ship an empty "note" and break the document's own contract. +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 { + files := make([]JSONFile, 0, len(chains)) + for _, ch := range chains { + steps := make([]JSONStep, 0, len(ch.Steps)) + for _, s := range ch.Steps { + steps = append(steps, JSONStep{ + Action: actionNames[s.Kind], + Rule: s.Rule, + Src: s.Src, + Dst: s.Dst, + Displaces: s.Displaces, + Reason: s.Reason, + Skip: s.Skip, + }) + } + files = append(files, JSONFile{ + Rel: ch.File.Rel, + Size: ch.File.Size, + ModTime: ch.File.ModTime, + Steps: steps, + Warnings: ch.Warnings, + }) + } + return JSONDir{Name: name, Root: root, Files: files, Warnings: warnings} +} diff --git a/internal/plan/json_test.go b/internal/plan/json_test.go new file mode 100644 index 0000000..86c181c --- /dev/null +++ b/internal/plan/json_test.go @@ -0,0 +1,87 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package plan + +import ( + "encoding/json" + "strings" + "testing" + "time" +) + +func TestJSONDir(t *testing.T) { + chains := []Chain{{ + File: file("/r", "x.pdf"), + Steps: []Step{ + {Kind: Copy, Rule: "b", Src: "/r/x.pdf", Dst: "/backup/x.pdf", Reason: `content "acme ltd"`}, + {Kind: Move, Rule: "a", Src: "/r/x.pdf", Dst: "/r/W/x.pdf", Displaces: "/r/W/x.pdf"}, + {Kind: Rename, Rule: "r", Src: "/r/W/x.pdf", Dst: "/r/W/2026-x.pdf"}, + {Kind: Trash, Rule: "t", Src: "/r/W/2026-x.pdf"}, + {Kind: DeletePermanent, Rule: "z", Src: "/r/W/x.pdf", Skip: "deleted by rule a"}, + }, + 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)}}, "", " ") + if err != nil { + t.Fatal(err) + } + out := string(b) + for _, want := range []string{ + `"version": 1`, + `"note": "the shape of this document is unstable before krino 1.0"`, + `"name": "dl"`, + `"rel": "x.pdf"`, + `"action": "copy"`, + // D14: the reason a rule matched is carried into the JSON document too. + `"reason": "content \"acme ltd\""`, + // D7: rename and trash were previously covered only by inspection, so + // an edit garbling either name would have passed silently. + `"action": "rename"`, + `"action": "trash"`, + `"action": "move"`, + `"displaces": "/r/W/x.pdf"`, + `"action": "delete"`, + `"skip": "deleted by rule a"`, + } { + if !strings.Contains(out, want) { + t.Errorf("json lacks %s:\n%s", want, out) + } + } + if strings.Contains(out, `"dst": ""`) { + t.Error("empty dst must be omitted") + } + var round JSON + if err := json.Unmarshal(b, &round); err != nil { + t.Fatalf("does not round-trip: %v", err) + } + if !round.Dirs[0].Files[0].ModTime.Equal(time.Date(2026, 8, 15, 12, 0, 0, 0, time.UTC)) { + t.Errorf("mtime did not survive: %v", round.Dirs[0].Files[0].ModTime) + } +} + +func TestJSONDirEmptyStepsAndFilesAreArraysNotNull(t *testing.T) { + stepless := []Chain{{File: file("/r", "y.pdf")}} + dir := NewJSONDir("dl", "/r", stepless, nil) + b, err := json.Marshal(dir) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(b), `"steps":null`) { + t.Errorf("stepless file's steps must be [], not null: %s", b) + } + if !strings.Contains(string(b), `"steps":[]`) { + t.Errorf("stepless file's steps should marshal as []: %s", b) + } + + empty := NewJSONDir("dl", "/r", nil, nil) + b, err = json.Marshal(empty) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(b), `"files":null`) { + t.Errorf("empty dir's files must be [], not null: %s", b) + } + if !strings.Contains(string(b), `"files":[]`) { + t.Errorf("empty dir's files should marshal as []: %s", b) + } +} diff --git a/internal/plan/placeholder.go b/internal/plan/placeholder.go new file mode 100644 index 0000000..b947236 --- /dev/null +++ b/internal/plan/placeholder.go @@ -0,0 +1,155 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +// Package plan turns matched rules into concrete actions: placeholder +// expansion, chains, conflict resolution and the JSON plan representation. +package plan + +import ( + "errors" + "fmt" + "strconv" + "strings" + "time" +) + +// Facts are the values a placeholder can reference. +type Facts struct { + Name string // the file's current base name + Captures []string // [0] whole match, [1:] groups, from the rule's first true name test + ModTime time.Time + Now time.Time +} + +// splitExt splits name on its last dot, which does not count when it is the +// first character: "a.tar.gz" -> "a.tar", ".gz"; ".bashrc" -> ".bashrc", "". +func splitExt(name string) (stem, ext string) { + i := strings.LastIndexByte(name, '.') + if i <= 0 { + return name, "" + } + return name[:i], name[i:] +} + +// Expand replaces every placeholder in s. It returns an error naming the +// first placeholder it could not expand. It scans s once: "{{" and "}}" +// each emit one literal brace, and any other "{" opens a placeholder that +// runs to the next "}". +func Expand(s string, f Facts) (string, error) { + var b strings.Builder + for i := 0; i < len(s); { + switch { + case s[i] == '{' && i+1 < len(s) && s[i+1] == '{': + b.WriteByte('{') + i += 2 + case s[i] == '}' && i+1 < len(s) && s[i+1] == '}': + b.WriteByte('}') + i += 2 + case s[i] == '{': + end := strings.IndexByte(s[i+1:], '}') + if end < 0 { + return "", errors.New("unclosed placeholder") + } + body := s[i+1 : i+1+end] + val, err := expandOne(body, f) + if err != nil { + return "", err + } + // D2: val is appended with WriteString, after the scan has + // already moved past this placeholder - it is never re-passed + // through this loop, so a "}}" or "{{" inside a capture's own + // text is never collapsed the way one written in the template + // itself would be. A refactor to scan-then-replace would + // silently undo this; TestExpandCaptureNotRescanned pins it. + b.WriteString(val) + i += end + 2 + default: + b.WriteByte(s[i]) + i++ + } + } + return b.String(), nil +} + +// expandOne expands the body of a single {...} placeholder (without the +// braces) into its replacement text. +func expandOne(body string, f Facts) (string, error) { + switch body { + case "name": + return f.Name, nil + case "stem": + stem, _ := splitExt(f.Name) + return stem, nil + case "ext": + _, ext := splitExt(f.Name) + return ext, nil + } + + if verb, format, ok := strings.Cut(body, ":"); ok { + switch verb { + case "mtime": + return strftime(verb, format, f.ModTime) + case "now": + return strftime(verb, format, f.Now) + } + return "", fmt.Errorf("unknown placeholder {%s}", body) + } + + if n, err := strconv.Atoi(body); err == nil { + if n == 0 { + return "", errors.New("capture groups are numbered from 1") + } + // D3: spec §7.3 defines the syntax as {1}...{9}, the same window + // MaxIndex enforces; without this check {10} and up bypass + // checkCaptures entirely (MaxIndex never sees them as capture + // uses) and fail only here, at expansion time. + if n > 9 { + return "", fmt.Errorf("unknown placeholder {%s}", body) + } + if n < 0 || n >= len(f.Captures) { + return "", fmt.Errorf("no capture group %d", n) + } + return f.Captures[n], nil + } + + return "", fmt.Errorf("unknown placeholder {%s}", body) +} + +// strftime renders a strftime subset (%Y %m %d %H %M %S %j %%) of t. verb is +// the placeholder's own verb ("mtime" or "now"), named in error messages so +// they point at what the config author actually wrote (B3) instead of +// hardcoding "mtime" for a {now:...} format error. +func strftime(verb, format string, t time.Time) (string, error) { + var b strings.Builder + for i := 0; i < len(format); i++ { + c := format[i] + if c != '%' { + b.WriteByte(c) + continue + } + i++ + if i >= len(format) { + return "", fmt.Errorf("unknown time format %%%c in {%s:...}", format[i-1], verb) + } + switch format[i] { + case 'Y': + fmt.Fprintf(&b, "%04d", t.Year()) + case 'm': + fmt.Fprintf(&b, "%02d", int(t.Month())) + case 'd': + fmt.Fprintf(&b, "%02d", t.Day()) + case 'H': + fmt.Fprintf(&b, "%02d", t.Hour()) + case 'M': + fmt.Fprintf(&b, "%02d", t.Minute()) + case 'S': + fmt.Fprintf(&b, "%02d", t.Second()) + case 'j': + fmt.Fprintf(&b, "%03d", t.YearDay()) + case '%': + b.WriteByte('%') + default: + return "", fmt.Errorf("unknown time format %%%c in {%s:...}", format[i], verb) + } + } + return b.String(), nil +} diff --git a/internal/plan/placeholder_test.go b/internal/plan/placeholder_test.go new file mode 100644 index 0000000..e155430 --- /dev/null +++ b/internal/plan/placeholder_test.go @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package plan + +import ( + "strings" + "testing" + "time" +) + +func facts() Facts { + return Facts{ + Name: "Invoice_2026-08.pdf", + Captures: []string{"2026-08", "2026", "a}}b"}, + ModTime: time.Date(2026, 8, 15, 14, 30, 45, 0, time.UTC), + Now: time.Date(2026, 9, 12, 9, 0, 0, 0, time.UTC), + } +} + +func TestExpand(t *testing.T) { + tests := []struct{ in, want string }{ + {"Work/Acme", "Work/Acme"}, + {"{name}", "Invoice_2026-08.pdf"}, + {"{stem}", "Invoice_2026-08"}, + {"{ext}", ".pdf"}, + {"{stem}{ext}", "Invoice_2026-08.pdf"}, + {"Work/{mtime:%Y}", "Work/2026"}, + {"{mtime:%Y-%m-%d}", "2026-08-15"}, + {"{mtime:%H%M%S}", "143045"}, + {"{mtime:%j}", "227"}, + {"{now:%Y-%m-%d}", "2026-09-12"}, + {"{1}", "2026"}, + // D2: an expanded value that itself contains "}}" must reach the + // output unchanged. Expand's single-pass scanner jumps past a + // placeholder's closing brace, so written bytes are never re-scanned; + // a refactor to scan-then-replace would silently re-collapse them. + {"{2}", "a}}b"}, + {"{{literal}}", "{literal}"}, + {"100{{%}}", "100{%}"}, + {"{mtime:%Y%%}", "2026%"}, + {"Photos/{mtime:%Y}/{stem}-{1}{ext}", "Photos/2026/Invoice_2026-08-2026.pdf"}, + } + for _, tt := range tests { + got, err := Expand(tt.in, facts()) + if err != nil || got != tt.want { + t.Errorf("Expand(%q) = %q, %v; want %q", tt.in, got, err, tt.want) + } + } +} + +func TestExpandNoExtension(t *testing.T) { + f := facts() + f.Name = "README" + for in, want := range map[string]string{"{stem}": "README", "{ext}": ""} { + if got, err := Expand(in, f); err != nil || got != want { + t.Errorf("Expand(%q) on README = %q, %v; want %q", in, got, err, want) + } + } + f.Name = ".bashrc" + if got, _ := Expand("{stem}", f); got != ".bashrc" { + t.Errorf("dotfile stem = %q, want .bashrc", got) + } + if got, _ := Expand("{ext}", f); got != "" { + t.Errorf("dotfile ext = %q, want empty", got) + } + f.Name = "archive.tar.gz" + if got, _ := Expand("{stem}|{ext}", f); got != "archive.tar|.gz" { + t.Errorf("double extension = %q, want archive.tar|.gz", got) + } +} + +func TestExpandErrors(t *testing.T) { + tests := []struct{ in, want string }{ + {"{7}", "no capture group 7"}, + {"{0}", "capture groups are numbered from 1"}, + {"{whatever}", "unknown placeholder {whatever}"}, + {"{name", "unclosed placeholder"}, + {"{mtime:%Q}", "unknown time format %Q in {mtime:...}"}, + {"{mtime}", "unknown placeholder {mtime}"}, + // B3: the error must name the placeholder actually written ("now"), + // not hardcode "mtime" - the {mtime:%Q} case above passes either + // way, which is why that defect survived. + {"{now:%Q}", "unknown time format %Q in {now:...}"}, + // D3: {1}...{9} is the syntax (spec §7.3); {10} and up must be + // rejected the same way an unknown placeholder is. + {"{10}", "unknown placeholder {10}"}, + } + for _, tt := range tests { + _, err := Expand(tt.in, facts()) + if err == nil || !strings.Contains(err.Error(), tt.want) { + t.Errorf("Expand(%q) error = %v; want %q", tt.in, err, tt.want) + } + } +} diff --git a/internal/plan/step.go b/internal/plan/step.go new file mode 100644 index 0000000..e4f280f --- /dev/null +++ b/internal/plan/step.go @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package plan + +import ( + "fmt" + + "krino/internal/config" + "krino/internal/scan" +) + +// Kind is what a step does. +type Kind int + +const ( + Copy Kind = iota + Move + Rename + Trash // (delete) + DeletePermanent // (delete permanent) +) + +// String is for display only; Task 5's JSON representation defines its own +// action names. +func (k Kind) String() string { + switch k { + case Copy: + return "copy" + case Move: + return "move" + case Rename: + return "rename" + case Trash: + return "trash" + case DeletePermanent: + return "DELETE permanently" + } + return fmt.Sprintf("Kind(%d)", int(k)) +} + +// Step is one action to carry out, already resolved to absolute paths. +type Step struct { + Kind Kind + Rule string // the rule that contributed it + Src string // absolute path the step reads from + Dst string // absolute path the file has after the step; "" for the two deletes + Reason string // the rule's match reasons, for display + Skip string // non-empty: this step will not run, and why + Conflict config.Conflict // the contributing rule's on-conflict policy + Displaces string // overwrite only: the existing file that must be trashed first +} + +// Chain is one file's steps, in order. +type Chain struct { + File scan.File + Steps []Step + Warnings []string +} + +// RuleMatch is one matching rule's contribution to a file's chain. +// internal/plan must not import internal/engine (engine imports plan), so +// the engine converts its own types into these. +type RuleMatch struct { + Name string + Actions []config.Action + Settings config.Resolved + Captures []string + Reasons []string +} -- cgit v1.3