feat(rules): build the scoped query in one place

A rule stores no scope. The hook supplies tag:new, a dry run supplies
nothing. The parentheses around the rule's own query are what stop a
disjunction of senders from escaping the scope and matching everything.

2 files changed, 48 insertions, 0 deletions

diff --git a/mailrules.py b/mailrules.py
index 8c65708..dbb80a5 100755
--- a/mailrules.py
+++ b/mailrules.py
@@ -126,6 +126,25 @@ def load(path=None):
     return store
 
 
+def scoped_query(rule, scope):
+    """The rule's query narrowed by `scope`, or the bare query when scope is
+    empty.
+
+    The parentheses are load-bearing. notmuch's `and` binds tighter than
+    `or`, so `tag:new and a or b` means `(tag:new and a) or b`: a rule that
+    is a disjunction of senders would escape its scope and match the whole
+    corpus. Do not remove them, and do not build this string anywhere else.
+    """
+    if not scope:
+        return rule.query
+    return f"{scope} and ({rule.query})"
+
+
+def tag_arguments(rule):
+    """The +tag/-tag arguments for `notmuch tag`, adds before removes."""
+    return [f"+{t}" for t in rule.add] + [f"-{t}" for t in rule.remove]
+
+
 def save(store, path=None):
     """Write the store atomically: a temp file in the same directory, then
     rename. Rename within a filesystem is atomic, so a concurrent reader sees
diff --git a/test_mailrules.py b/test_mailrules.py
index a3faa5e..b1f31c9 100755
--- a/test_mailrules.py
+++ b/test_mailrules.py
@@ -237,6 +237,35 @@ def test_save_creates_the_directory():
         assert json.loads(path.read_text()) == {"version": 1, "rules": []}
 
 
+def test_scoped_query_parenthesises_the_rule():
+    """Without the parentheses `tag:new and a or b` binds as
+    `(tag:new and a) or b`, and the rule matches every message in the corpus
+    satisfying b rather than only new arrivals. Several real rules are a
+    disjunction of senders, so this is the difference between tagging four
+    messages and tagging four thousand."""
+    rule = mailrules.Rule(
+        id="disjunction",
+        query="from:a@example.com or from:b@example.com",
+        add=["promo"])
+    assert mailrules.scoped_query(rule, "tag:new") == (
+        "tag:new and (from:a@example.com or from:b@example.com)")
+
+
+def test_scoped_query_with_no_scope_is_the_bare_query():
+    """A dry run counts against the whole corpus, which is what makes the
+    same rule answer 'what would this tag on arrival' and 'what does this
+    match in all my mail'."""
+    rule = mailrules.Rule(id="x", query="from:a@example.com", add=["y"])
+    assert mailrules.scoped_query(rule, None) == "from:a@example.com"
+    assert mailrules.scoped_query(rule, "") == "from:a@example.com"
+
+
+def test_tag_arguments():
+    rule = mailrules.Rule(id="x", query="from:a@example.com",
+                          add=["one", "two"], remove=["three"])
+    assert mailrules.tag_arguments(rule) == ["+one", "+two", "-three"]
+
+
 def run_all():
     for name, fn in sorted(globals().items()):
         if name.startswith("test_") and callable(fn):