Let a baseline entry say who excused it and why

docs/REFERENCE.md

@@ -72,6 +72,44 @@ every git hook, and a repository whose own prose cites its issues would have
 every one of those citations refused — so the seam it belongs at is the command
 that publishes text to a forge, and only that one.
 
+### A baseline entry may be asked to say who excused it and why
+
+`files.baseline` names a file of repository-relative paths a rule excuses, and
+an entry that no longer matches is reported as stale -- an exemption that has
+stopped describing the tree is the rule switched off for that path.
+
+A baseline holds two different things and the format could only express one:
+
+* **debt** -- eight modules awaiting the same migration. One reason at the top
+  of the file covers every entry, and the file's header is the right place for
+  it.
+* **exceptions** -- the places a rule is simply wrong. `.ljust(` building a
+  five-column table should take the dependency; `.ljust(` building a two-column
+  key/value list is correct and a table would read worse. No pattern separates
+  those, so the entry excusing the second has to carry the judgement, and the
+  whole line was the path.
+
+So an entry may be signed:
+
+```text
+# the places this rule is wrong
+src/cli/top.py | alice | a two-column key/value list; a table reads worse
+```
+
+`path | owner | reason`, with `|` as the separator -- whitespace already
+separates the size baseline's count and a path may hold it, and `#` at line
+start already means a comment.
+
+Set `baselines_signed = true` at the top of the policy to require it. Off by
+default, which is what every existing baseline file already is; a repository
+turns it on when its baselines stop being one homogeneous debt. Unsigned
+entries are then reported at the same tier as stale ones, and for the same
+reason: both are a baseline that has stopped recording a decision somebody made.
+
+A signature is an addition to the record and not a way out of it. A reason says
+why an entry is there; it says nothing about whether it still needs to be, so a
+signed entry still goes stale.
+
 ### A rule may not be about its own declaration
 
 A policy file is a tracked file, so a rule's `regexp` and `require_regexp` are

src/config.rs

@@ -1480,6 +1480,23 @@ pub(crate) struct PolicyFile {
     pub rules: BTreeMap<String, Rule>,
     #[serde(default, rename = "shim")]
     pub shims: Vec<crate::shim::Shim>,
+    /// Whether every path-baseline entry must say who excused it and why.
+    ///
+    /// Off by default, and the default is not neutrality -- it is what every
+    /// existing baseline file already is. A repository turns it on when its
+    /// baselines stop being one homogeneous debt with a header explaining all
+    /// of it, and start holding entries that differ from each other. The
+    /// `.ljust(` case is the shape: one call site should take the dependency
+    /// and another is correct as it stands, and no pattern separates them, so
+    /// the entry that excuses the second has to carry the judgement or the
+    /// judgement is nowhere.
+    ///
+    /// Policy-level rather than per-rule, because "may an exemption be
+    /// anonymous" is one answer a repository gives once. Per-rule it would be a
+    /// setting every new baseline has to remember, which is the same as not
+    /// having it.
+    #[serde(default)]
+    pub baselines_signed: bool,
 }
 
 /// The ceiling on what one bundled set may install, declared in the set.
@@ -1558,6 +1575,9 @@ pub(crate) struct Policy {
     /// exit 2. See [`PolicyFile::private_owners_optional`].
     pub private_owners_optional: bool,
     pub redact_matches: bool,
+    /// Whether every path-baseline entry must be signed. See
+    /// [`PolicyFile::baselines_signed`].
+    pub baselines_signed: bool,
     pub allowed_scripts: Vec<String>,
     pub rules: Vec<Rule>,
     pub shims: Vec<crate::shim::Shim>,
@@ -2084,6 +2104,7 @@ pub(crate) fn load(root: &Path, policy_path: &Path) -> Result<Policy> {
         private_owners_from: file.private_owners_from.clone(),
         private_owners_optional: file.private_owners_optional,
         redact_matches: file.redact_matches,
+        baselines_signed: file.baselines_signed,
         allowed_scripts: file.allowed_scripts,
         rules,
         shims: file.shims,

src/scan.rs

@@ -240,9 +240,10 @@ impl<'a> Scan<'a> {
                 .filter(|path| baseline.contains(path))
                 .collect();
             hits.retain(|hit| !baseline.contains(normalize_rel(&hit.path)));
+            failures.extend(baseline.unsigned_failure(rule, self.policy.baselines_signed));
             failures.extend(stale_baseline_failure(
                 rule,
-                &baseline,
+                &baseline.paths,
                 &seen,
                 STALE_BASELINE,
             ));
@@ -492,9 +493,10 @@ impl<'a> Scan<'a> {
                 .filter(|path| baseline.contains(path))
                 .collect();
             hits.retain(|hit| !baseline.contains(normalize_rel(&hit.path)));
+            failures.extend(baseline.unsigned_failure(rule, self.policy.baselines_signed));
             failures.extend(stale_baseline_failure(
                 rule,
-                &baseline,
+                &baseline.paths,
                 &seen,
                 STALE_BASELINE,
             ));
@@ -540,9 +542,10 @@ impl<'a> Scan<'a> {
                 .iter()
                 .map(|path| normalize_rel(path).to_owned())
                 .collect();
+            failures.extend(baseline.unsigned_failure(rule, self.policy.baselines_signed));
             failures.extend(stale_baseline_failure(
                 rule,
-                &baseline,
+                &baseline.paths,
                 &still_missing,
                 STALE_REQUIRE_BASELINE,
             ));
@@ -1156,24 +1159,58 @@ impl<'a> Scan<'a> {
             .collect()
     }
 
-    /// A path-only baseline: one repository-relative path per line.
+    /// A path-only baseline: one repository-relative path per line, optionally
+    /// signed.
     ///
     /// Paths rather than counts, deliberately. A count baseline is stricter, but
     /// a reformat moves a match count without anything real changing, and a rule
     /// whose baseline churns on unrelated edits is one people stop reading. A
     /// listed path may get worse internally; what it cannot do is let a NEW path
     /// start.
-    fn load_path_baseline(&self, relative: Option<&str>) -> Result<BTreeSet<String>> {
+    ///
+    /// A line may carry a signature after the path:
+    ///
+    /// ```text
+    /// src/cli/top.py | alice | a two-column key/value list; a table reads worse
+    /// ```
+    ///
+    /// Optional here and required where the policy says so, because the two
+    /// things a baseline holds are not the same. A file listing eight modules
+    /// that have not been migrated yet needs one reason at the top and none per
+    /// line -- every entry is the same debt and the file's header says so. A
+    /// file listing the places a rule is WRONG needs one reason per line, and
+    /// nothing in the format could say it: the whole line was the path, so a
+    /// judgement about why this instance is the exception had nowhere to go
+    /// except a comment nothing associates with an entry.
+    ///
+    /// The separator is `|` rather than whitespace or `#`. Whitespace is the
+    /// size baseline's separator and a path may hold it; `#` at line start
+    /// already means a comment and overloading it would change what an existing
+    /// file means.
+    fn load_path_baseline(&self, relative: Option<&str>) -> Result<Baseline> {
         let Some(relative) = relative else {
-            return Ok(BTreeSet::new());
+            return Ok(Baseline::default());
         };
         let text = crate::error::read_to_string(&self.root.join(relative))?;
-        Ok(text
-            .lines()
-            .map(str::trim)
-            .filter(|line| !line.is_empty() && !line.starts_with('#'))
-            .map(|line| normalize_rel(line).to_owned())
-            .collect())
+        let mut baseline = Baseline {
+            file: relative.to_owned(),
+            ..Baseline::default()
+        };
+        for line in text.lines() {
+            let line = line.trim();
+            if line.is_empty() || line.starts_with('#') {
+                continue;
+            }
+            let mut parts = line.split('|').map(str::trim);
+            let path = normalize_rel(parts.next().unwrap_or_default()).to_owned();
+            let owner = parts.next().unwrap_or_default();
+            let reason = parts.next().unwrap_or_default();
+            if owner.is_empty() || reason.is_empty() {
+                baseline.unsigned.push(path.clone());
+            }
+            baseline.paths.insert(path);
+        }
+        Ok(baseline)
     }
 
     fn load_size_baseline(&self, relative: Option<&str>) -> Result<BTreeMap<String, u64>> {
@@ -1199,6 +1236,59 @@ impl<'a> Scan<'a> {
     }
 }
 
+/// One rule's baseline: the paths it excuses, and which of them are unsigned.
+///
+/// `file` is carried so a finding can name the file to edit. A report that says
+/// "three entries are unsigned" and not where they are is a report whose reader
+/// has to go looking for the thing it just read.
+#[derive(Debug, Default)]
+struct Baseline {
+    file: String,
+    paths: BTreeSet<String>,
+    unsigned: Vec<String>,
+}
+
+impl Baseline {
+    fn is_empty(&self) -> bool {
+        self.paths.is_empty()
+    }
+
+    fn contains(&self, path: &str) -> bool {
+        self.paths.contains(path)
+    }
+
+    /// The entries carrying no owner and reason, when the policy asks for them.
+    ///
+    /// A finding rather than a load refusal, because a baseline is read here
+    /// and not at load: the path comes from the rule and the file from the
+    /// tree, and neither exists as text the loader has seen. It sits at the
+    /// same tier as a stale entry for the same reason -- both are a baseline
+    /// that has stopped describing a decision somebody made.
+    fn unsigned_failure(&self, rule: &Rule, required: bool) -> Vec<Failure> {
+        if !required || self.unsigned.is_empty() {
+            return Vec::new();
+        }
+        let body = self
+            .unsigned
+            .iter()
+            .map(|path| format!("{path}: no owner and reason"))
+            .collect::<Vec<String>>()
+            .join("\n");
+        vec![Failure::new(
+            format!("{} (unsigned baseline)", rule.id),
+            format!(
+                "This policy requires every baseline entry to say who excused it and why, and \
+                 these say neither. Write them as `path | owner | reason` in {}.\n\nA path on \
+                 its own records that a rule was switched off there and nothing about the \
+                 judgement behind it -- which is the difference between debt somebody is \
+                 carrying and a finding somebody silenced.",
+                self.file
+            ),
+            body,
+        )]
+    }
+}
+
 fn stale_baseline_failure(
     rule: &Rule,
     baseline: &BTreeSet<String>,

tests/scan_cli.rs

@@ -482,6 +482,113 @@ fn a_baselined_path_is_allowed_and_a_new_one_is_not() {
     assert!(!text.contains("old.txt:"), "{text}");
 }
 
+#[test]
+fn an_unsigned_baseline_entry_is_reported_where_the_policy_asks_for_signatures() {
+    // A baseline holds two different things and the format could only express
+    // one. Eight modules awaiting the same migration need one reason at the top
+    // of the file. A list of the places a rule is WRONG needs one reason per
+    // line -- and until now the whole line was the path, so the judgement had
+    // nowhere to go but a comment nothing associates with an entry.
+    let root = workspace();
+    write(
+        &root,
+        "policy/principles.toml",
+        r#"
+        baselines_signed = true
+
+        [rule.no-todo]
+        message = "no TODO"
+        regexp = 'TODO'
+
+        [rule.no-todo.files]
+        exclude = ["policy/**"]
+        baseline = "policy/todo-baseline.txt"
+"#,
+    );
+    write(
+        &root,
+        "policy/todo-baseline.txt",
+        "# grandfathered\nold.txt\n",
+    );
+    write(&root, "old.txt", "TODO: ancient\n");
+
+    let output = scan(&root);
+    assert_eq!(code(&output), 1, "{}", stderr(&output));
+    let text = stderr(&output);
+    assert!(text.contains("no-todo (unsigned baseline)"), "{text}");
+    assert!(text.contains("old.txt: no owner and reason"), "{text}");
+    // It names the file to edit: a report that says three entries are unsigned
+    // and not where they are sends its reader looking.
+    assert!(text.contains("policy/todo-baseline.txt"), "{text}");
+
+    // Signed, it passes -- and the entry still suppresses what it excused.
+    write(
+        &root,
+        "policy/todo-baseline.txt",
+        "# grandfathered\nold.txt | alice | the tracker this cites was closed; text stays\n",
+    );
+    assert_eq!(code(&scan(&root)), 0);
+}
+
+#[test]
+fn an_unsigned_baseline_is_fine_where_the_policy_does_not_ask() {
+    // The default is not neutrality, it is what every existing baseline file
+    // already is. Turning this on is a repository saying its baselines have
+    // stopped being one homogeneous debt.
+    let root = workspace();
+    write(
+        &root,
+        "policy/principles.toml",
+        r#"
+        [rule.no-todo]
+        message = "no TODO"
+        regexp = 'TODO'
+
+        [rule.no-todo.files]
+        exclude = ["policy/**"]
+        baseline = "policy/todo-baseline.txt"
+"#,
+    );
+    write(&root, "policy/todo-baseline.txt", "old.txt\n");
+    write(&root, "old.txt", "TODO: ancient\n");
+    assert_eq!(code(&scan(&root)), 0);
+}
+
+#[test]
+fn a_signed_entry_still_goes_stale_when_it_stops_describing_the_tree() {
+    // The signature is an addition to the record, not a way out of it. A
+    // reason explains why an entry is there; it says nothing about whether it
+    // still needs to be.
+    let root = workspace();
+    write(
+        &root,
+        "policy/principles.toml",
+        r#"
+        baselines_signed = true
+
+        [rule.no-todo]
+        message = "no TODO"
+        regexp = 'TODO'
+
+        [rule.no-todo.files]
+        exclude = ["policy/**"]
+        baseline = "policy/todo-baseline.txt"
+"#,
+    );
+    write(
+        &root,
+        "policy/todo-baseline.txt",
+        "paid.txt | alice | pre-existing, being migrated\n",
+    );
+    write(&root, "paid.txt", "clean now\n");
+
+    let output = scan(&root);
+    assert_eq!(code(&output), 1, "{}", stderr(&output));
+    let text = stderr(&output);
+    assert!(text.contains("no-todo (stale baseline)"), "{text}");
+    assert!(!text.contains("unsigned baseline"), "{text}");
+}
+
 #[test]
 fn a_baseline_entry_that_no_longer_matches_is_reported_as_stale() {
     let root = workspace();