(matched) is unknown after an undecided rule, so a catch-all leaves an unreadable file alone

6 files changed, 82 insertions, 14 deletions

diff --git a/internal/cond/eval.go b/internal/cond/eval.go
index c9f7f75..31c54e3 100644
--- a/internal/cond/eval.go
+++ b/internal/cond/eval.go
@@ -24,7 +24,11 @@ type Facts interface {
 	// -1 when it contains none.
 	ContentContains(opt Options, keywords []string) (int, error)
 	Duplicate(dirs []string) (original string, ok bool, err error)
-	Matched() bool // an earlier rule matched this file
+	// Matched reports whether an earlier rule matched this file, and whether
+	// an earlier rule could not be decided (its condition was unknown): with
+	// no match and an undecided rule, (matched) is unknown (plan 11 review
+	// L6).
+	Matched() (matched, undecided bool)
 }
 
 // Result is the outcome of evaluating a Cond against one file's Facts.
@@ -154,6 +158,9 @@ func (c *Cond) eval(n *node, f Facts, ctx *evalCtx, negated bool) (tri, []string
 		}
 		return yes, []string{"not " + label}
 	default:
+		if m, undecided := f.Matched(); n.kind == kMatched && !m && undecided {
+			return unknown, nil
+		}
 		ok, reason, warn, caps := c.evalLeaf(n, f)
 		ctx.warn(warn)
 		if warn != "" && n.kind == kContent {
@@ -258,7 +265,7 @@ func (c *Cond) evalLeaf(n *node, f Facts) (ok bool, reason, warn string, caps []
 		return false, "", "", nil
 
 	case kMatched:
-		if f.Matched() {
+		if m, _ := f.Matched(); m {
 			return true, "matched", "", nil
 		}
 		return false, "", "", nil
@@ -348,7 +355,7 @@ func (c *Cond) explain(n *node, f Facts) *Trace {
 	default:
 		ok, _, warn, _ := c.evalLeaf(n, f)
 		t := &Trace{Label: n.label, Value: ok, Err: warn}
-		if warn != "" && n.kind == kContent {
+		if m, undecided := f.Matched(); (warn != "" && n.kind == kContent) || (n.kind == kMatched && !m && undecided) {
 			t.set(unknown)
 		}
 		return t
diff --git a/internal/cond/eval_test.go b/internal/cond/eval_test.go
index 7d796a1..9352b82 100644
--- a/internal/cond/eval_test.go
+++ b/internal/cond/eval_test.go
@@ -21,6 +21,7 @@ type fake struct {
 	size           int64
 	age            time.Duration
 	matched        bool
+	matchedUnknown bool
 	dupOrig        string
 	dupOK          bool
 	contentCalls   int
@@ -33,10 +34,10 @@ func (f *fake) Rel() string {
 	}
 	return f.name
 }
-func (f *fake) Size() int64        { return f.size }
-func (f *fake) ModTime() time.Time { return now.Add(-f.age) }
-func (f *fake) Now() time.Time     { return now }
-func (f *fake) Matched() bool      { return f.matched }
+func (f *fake) Size() int64           { return f.size }
+func (f *fake) ModTime() time.Time    { return now.Add(-f.age) }
+func (f *fake) Now() time.Time        { return now }
+func (f *fake) Matched() (bool, bool) { return f.matched, f.matchedUnknown }
 func (f *fake) ContentContains(opt Options, keywords []string) (int, error) {
 	f.contentCalls++
 	if f.rawErr != nil {
@@ -284,3 +285,18 @@ func TestUnreadableContentIsUnknown(t *testing.T) {
 		}
 	}
 }
+
+// TestMatchedIsUnknownAfterAnUnknownRule: when no earlier rule matched but
+// one could not be decided, (matched) is unknown, so a later
+// (not (matched)) does not act on a file krino could not read (plan 11
+// review L6).
+func TestMatchedIsUnknownAfterAnUnknownRule(t *testing.T) {
+	f := &fake{name: "a.docx", matchedUnknown: true}
+	if r := eval(t, `(not (matched))`, Options{}, f); r.Match || !r.Unreadable {
+		t.Errorf("(not (matched)): Match %v Unreadable %v; want false, true", r.Match, r.Unreadable)
+	}
+	f = &fake{name: "a.docx", matched: true, matchedUnknown: true}
+	if r := eval(t, `(matched)`, Options{}, f); !r.Match {
+		t.Errorf("(matched) after a rule that did match: %+v, want true", r)
+	}
+}
diff --git a/internal/engine/engine_test.go b/internal/engine/engine_test.go
index c38c1fc..9de3ea4 100644
--- a/internal/engine/engine_test.go
+++ b/internal/engine/engine_test.go
@@ -53,7 +53,7 @@ func (f fakeFacts) ModTime() time.Time                                  { return
 func (f fakeFacts) Now() time.Time                                      { return time.Time{} }
 func (f fakeFacts) ContentContains(cond.Options, []string) (int, error) { return -1, nil }
 func (f fakeFacts) Duplicate([]string) (string, bool, error)            { return "", false, nil }
-func (f fakeFacts) Matched() bool                                       { return false }
+func (f fakeFacts) Matched() (bool, bool)                               { return false, false }
 
 var _ cond.Facts = fakeFacts{}
 
diff --git a/internal/engine/exclude_test.go b/internal/engine/exclude_test.go
index 9aafd7f..3398b07 100644
--- a/internal/engine/exclude_test.go
+++ b/internal/engine/exclude_test.go
@@ -512,3 +512,39 @@ func TestExplainAgreesWithMatchOnADuplicateExclude(t *testing.T) {
 		}
 	}
 }
+
+// TestNotMatchedAfterAnUnknownRuleDoesNotAct: a partly read document an
+// earlier content rule could not decide is not caught by a later
+// (not (matched)) catch-all (plan 11 review L6).
+func TestNotMatchedAfterAnUnknownRuleDoesNotAct(t *testing.T) {
+	h, dl := excludeTree(t, map[string]string{})
+	os.MkdirAll(dl, 0o755)
+	partDocx(t, filepath.Join(dl, "part.docx"), "good body text")
+	main := writeConfig(t, h, `(include "dl")`, map[string]string{"dl": `
+(path "~/dl")
+(rule "acme" (when (content "acme ltd")) (move "Acme"))
+(rule "rest" (when (not (matched))) (move "Unsorted"))
+`})
+	e, errs := Load(main)
+	if len(errs) > 0 {
+		t.Fatal(errs)
+	}
+	r, err := e.Match(context.Background(), e.Dirs[0])
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, fm := range r.Matched {
+		if fm.File.Rel == "part.docx" {
+			t.Errorf("part.docx matched %d rules; the catch-all must not take a file an earlier rule could not decide", len(fm.Rules))
+		}
+	}
+	x, err := e.Explain(context.Background(), filepath.Join(dl, "part.docx"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, rt := range x.Rules {
+		if rt.Match {
+			t.Errorf("explain: rule %s matches", rt.Rule.Name)
+		}
+	}
+}
diff --git a/internal/engine/facts.go b/internal/engine/facts.go
index f8fde89..89bd71b 100644
--- a/internal/engine/facts.go
+++ b/internal/engine/facts.go
@@ -132,6 +132,9 @@ type facts struct {
 	file scan.File
 
 	matched bool
+	// undecided: an earlier rule's condition was unknown (content krino
+	// could not read), so (matched) is unknown while none has matched.
+	undecided bool
 
 	contentDone bool            // extraction was attempted
 	contentErr  error           // why it failed
@@ -146,12 +149,12 @@ func newFacts(run *matchRun, file scan.File) *facts {
 	return &facts{run: run, file: file}
 }
 
-func (f *facts) Name() string       { return f.file.Name }
-func (f *facts) Rel() string        { return f.file.Rel }
-func (f *facts) Size() int64        { return f.file.Size }
-func (f *facts) ModTime() time.Time { return f.file.ModTime }
-func (f *facts) Now() time.Time     { return f.run.now }
-func (f *facts) Matched() bool      { return f.matched }
+func (f *facts) Name() string          { return f.file.Name }
+func (f *facts) Rel() string           { return f.file.Rel }
+func (f *facts) Size() int64           { return f.file.Size }
+func (f *facts) ModTime() time.Time    { return f.file.ModTime }
+func (f *facts) Now() time.Time        { return f.run.now }
+func (f *facts) Matched() (bool, bool) { return f.matched, f.undecided }
 
 // ContentContains answers a content test (spec §6.1). A file above
 // max-read is never read, cached or not. Before the file has been
diff --git a/internal/engine/match.go b/internal/engine/match.go
index c252596..2ef3ae1 100644
--- a/internal/engine/match.go
+++ b/internal/engine/match.go
@@ -169,6 +169,9 @@ func evalFile(run *matchRun, file scan.File) FileMatch {
 		for _, w := range res.Warnings {
 			fm.Warnings = append(fm.Warnings, r.Name+": "+w)
 		}
+		if res.Unreadable {
+			f.undecided = true
+		}
 		if !res.Match {
 			continue
 		}
@@ -316,6 +319,9 @@ func (e *Engine) Explain(ctx context.Context, path string) (*Explanation, error)
 		}
 		trace := r.Cond.Explain(f)
 		match := trace.Value
+		if trace.Unknown {
+			f.undecided = true
+		}
 		if match {
 			f.matched = true
 			matched = append(matched, RuleMatch{Rule: r})