Implement Rule, RulePlugin, and existing achievements

herostratus/src/observer/impls/empty_commit.rs

@@ -1 +1,107 @@
+use std::mem::Discriminant;
 
+use crate::observer::observation::Observation;
+use crate::observer::observer::{DiffAction, Observer};
+use crate::observer::observer_factory::ObserverFactory;
+
+/// Emits [Observation::EmptyCommit] when a non-merge commit introduces no file changes.
+///
+/// Merge commits are excluded -- an empty diff is expected and normal for them.
+#[derive(Default)]
+pub struct EmptyCommitObserver {
+    is_merge: bool,
+    found_any_change: bool,
+}
+
+inventory::submit!(ObserverFactory::new::<EmptyCommitObserver>());
+
+impl Observer for EmptyCommitObserver {
+    fn emits(&self) -> Discriminant<Observation> {
+        Observation::EMPTY_COMMIT
+    }
+
+    fn is_interested_in_diff(&self) -> bool {
+        true
+    }
+
+    fn on_commit(
+        &mut self,
+        commit: &gix::Commit,
+        _repo: &gix::Repository,
+    ) -> eyre::Result<Option<Observation>> {
+        self.is_merge = commit.parent_ids().count() > 1;
+        Ok(None)
+    }
+
+    fn on_diff_start(&mut self) -> eyre::Result<()> {
+        self.found_any_change = false;
+        Ok(())
+    }
+
+    fn on_diff_change(
+        &mut self,
+        _change: &gix::object::tree::diff::ChangeDetached,
+        _repo: &gix::Repository,
+    ) -> eyre::Result<DiffAction> {
+        if self.is_merge {
+            return Ok(DiffAction::Cancel);
+        }
+        self.found_any_change = true;
+        // One change is enough to know it's not empty
+        Ok(DiffAction::Cancel)
+    }
+
+    fn on_diff_end(&mut self) -> eyre::Result<Option<Observation>> {
+        if self.is_merge || self.found_any_change {
+            return Ok(None);
+        }
+        Ok(Some(Observation::EmptyCommit))
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use herostratus_tests::fixtures::repository;
+
+    use super::*;
+    use crate::observer::impls::test_helpers::observe_all;
+
+    #[test]
+    fn empty_commit_detected() {
+        // The repository::Builder creates empty commits by default (no file changes)
+        let repo = repository::Builder::new()
+            .commit("first")
+            .commit("empty commit")
+            .build()
+            .unwrap();
+        let observations = observe_all(&repo, EmptyCommitObserver::default());
+        // Both commits are empty (no file changes)
+        assert_eq!(
+            observations,
+            [Observation::EmptyCommit, Observation::EmptyCommit]
+        );
+    }
+
+    #[test]
+    fn non_empty_commit_not_detected() {
+        let repo = repository::Builder::new()
+            .commit("add file")
+            .file("hello.txt", b"hello")
+            .build()
+            .unwrap();
+        let observations = observe_all(&repo, EmptyCommitObserver::default());
+        assert!(observations.is_empty());
+    }
+
+    #[test]
+    fn mixed_commits() {
+        let repo = repository::Builder::new()
+            .commit("add file")
+            .file("hello.txt", b"hello")
+            .commit("empty follow-up")
+            .build()
+            .unwrap();
+        let observations = observe_all(&repo, EmptyCommitObserver::default());
+        assert_eq!(observations, [Observation::EmptyCommit]);
+    }
+}

herostratus/src/observer/impls/fixup.rs

@@ -1 +1,105 @@
+use std::mem::Discriminant;
 
+use crate::observer::observation::Observation;
+use crate::observer::observer::Observer;
+use crate::observer::observer_factory::ObserverFactory;
+
+const FIXUP_PREFIXES: &[&str] = &[
+    "fixup!", "squash!", "amend!", "WIP", "TODO", "FIXME", "DROPME",
+    // Avoid false positives by accepting false negatives. Of all these patterns, "wip" is the one
+    // that's most likely to be a part of a real word.
+    "wip:", "todo", "fixme", "dropme",
+];
+
+/// Emits [Observation::Fixup] when the commit subject starts with a fixup/squash/amend/WIP/etc
+/// prefix.
+#[derive(Default)]
+pub struct FixupObserver;
+
+inventory::submit!(ObserverFactory::new::<FixupObserver>());
+
+impl Observer for FixupObserver {
+    fn emits(&self) -> Discriminant<Observation> {
+        Observation::FIXUP
+    }
+
+    fn on_commit(
+        &mut self,
+        commit: &gix::Commit,
+        _repo: &gix::Repository,
+    ) -> eyre::Result<Option<Observation>> {
+        let msg = commit.message()?;
+        let found = FIXUP_PREFIXES
+            .iter()
+            .any(|p| msg.title.starts_with(p.as_bytes()));
+        Ok(found.then_some(Observation::Fixup))
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use herostratus_tests::fixtures::repository;
+
+    use super::*;
+    use crate::observer::impls::test_helpers::observe_all;
+
+    #[test]
+    fn fixup_prefix() {
+        let repo = repository::Builder::new()
+            .commit("fixup! some commit")
+            .build()
+            .unwrap();
+        let observations = observe_all(&repo, FixupObserver);
+        assert_eq!(observations, [Observation::Fixup]);
+    }
+
+    #[test]
+    fn squash_prefix() {
+        let repo = repository::Builder::new()
+            .commit("squash! some commit")
+            .build()
+            .unwrap();
+        let observations = observe_all(&repo, FixupObserver);
+        assert_eq!(observations, [Observation::Fixup]);
+    }
+
+    #[test]
+    fn normal_commit() {
+        let repo = repository::Builder::new()
+            .commit("Normal commit message")
+            .build()
+            .unwrap();
+        let observations = observe_all(&repo, FixupObserver);
+        assert!(observations.is_empty());
+    }
+
+    #[test]
+    fn case_sensitive_wip() {
+        let repo = repository::Builder::new()
+            .commit("WIP something")
+            .build()
+            .unwrap();
+        let observations = observe_all(&repo, FixupObserver);
+        assert_eq!(observations, [Observation::Fixup]);
+    }
+
+    #[test]
+    fn lowercase_wip_needs_colon() {
+        let repo = repository::Builder::new()
+            .commit("wip: something")
+            .build()
+            .unwrap();
+        let observations = observe_all(&repo, FixupObserver);
+        assert_eq!(observations, [Observation::Fixup]);
+    }
+
+    #[test]
+    fn wip_without_colon_no_match() {
+        let repo = repository::Builder::new()
+            .commit("wipe out old data")
+            .build()
+            .unwrap();
+        let observations = observe_all(&repo, FixupObserver);
+        assert!(observations.is_empty());
+    }
+}

herostratus/src/observer/impls/mod.rs

@@ -3,3 +3,41 @@ mod fixup;
 mod non_unicode;
 mod subject_length;
 mod whitespace_only;
+
+#[cfg(test)]
+pub(crate) mod test_helpers {
+    use herostratus_tests::fixtures::repository::TempRepository;
+
+    use crate::git::mailmap::MailmapResolver;
+    use crate::observer::{Observation, Observer, ObserverData, ObserverEngine};
+
+    /// Run a single observer against all commits in the repo (oldest first) and collect the
+    /// observations it emits.
+    pub fn observe_all(
+        repo: &TempRepository,
+        observer: impl Observer + 'static,
+    ) -> Vec<Observation> {
+        let mailmap = MailmapResolver::new(gix::mailmap::Snapshot::default(), None, None).unwrap();
+        let observers: Vec<Box<dyn Observer>> = vec![Box::new(observer)];
+        let mut engine = ObserverEngine::new(&repo.repo, observers, mailmap).unwrap();
+
+        let head = crate::git::rev::parse("HEAD", &repo.repo).unwrap();
+        let oids: Vec<_> = crate::git::rev::walk(head, &repo.repo)
+            .unwrap()
+            .map(|r| r.unwrap())
+            .collect();
+        // Walk returns newest-first; reverse to process oldest-first
+        let oids: Vec<_> = oids.into_iter().rev().collect();
+
+        let (tx, rx) = std::sync::mpsc::channel();
+        engine.run(oids, &tx).unwrap();
+        drop(tx);
+
+        rx.iter()
+            .filter_map(|msg| match msg {
+                ObserverData::Observation(obs) => Some(obs),
+                _ => None,
+            })
+            .collect()
+    }
+}

herostratus/src/observer/impls/non_unicode.rs

@@ -1 +1,32 @@
+use std::mem::Discriminant;
 
+use crate::observer::observation::Observation;
+use crate::observer::observer::Observer;
+use crate::observer::observer_factory::ObserverFactory;
+
+/// Emits [Observation::NonUnicodeMessage] when the raw commit message contains bytes that are not
+/// valid UTF-8.
+#[derive(Default)]
+pub struct NonUnicodeObserver;
+
+inventory::submit!(ObserverFactory::new::<NonUnicodeObserver>());
+
+impl Observer for NonUnicodeObserver {
+    fn emits(&self) -> Discriminant<Observation> {
+        Observation::NON_UNICODE_MESSAGE
+    }
+
+    fn on_commit(
+        &mut self,
+        commit: &gix::Commit,
+        _repo: &gix::Repository,
+    ) -> eyre::Result<Option<Observation>> {
+        let bytes = commit.message_raw_sloppy();
+        let is_non_utf8 = std::str::from_utf8(bytes).is_err();
+        Ok(is_non_utf8.then_some(Observation::NonUnicodeMessage))
+    }
+}
+
+// It's not possible to create a commit containing non-unicode bytes from gix or git2, so there's
+// no unit test here. There's an integration test against a hand-crafted branch with non-unicode
+// bytes in the commit message. See h004_non_unicode.rs for details.

herostratus/src/observer/impls/subject_length.rs

@@ -1 +1,61 @@
+use std::mem::Discriminant;
 
+use crate::observer::observation::Observation;
+use crate::observer::observer::Observer;
+use crate::observer::observer_factory::ObserverFactory;
+
+/// Emits [Observation::SubjectLength] for every commit, carrying the byte length of the subject
+/// line.
+#[derive(Default)]
+pub struct SubjectLengthObserver;
+
+inventory::submit!(ObserverFactory::new::<SubjectLengthObserver>());
+
+impl Observer for SubjectLengthObserver {
+    fn emits(&self) -> Discriminant<Observation> {
+        Observation::SUBJECT_LENGTH
+    }
+
+    fn on_commit(
+        &mut self,
+        commit: &gix::Commit,
+        _repo: &gix::Repository,
+    ) -> eyre::Result<Option<Observation>> {
+        let msg = commit.message()?;
+        // Number of bytes, not number of characters, but that's fine for our purposes
+        let length = msg.title.len();
+        Ok(Some(Observation::SubjectLength { length }))
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use herostratus_tests::fixtures::repository;
+
+    use super::*;
+    use crate::observer::impls::test_helpers::observe_all;
+
+    #[test]
+    fn emits_length() {
+        let repo = repository::Builder::new().commit("Hello").build().unwrap();
+        let observations = observe_all(&repo, SubjectLengthObserver);
+        assert_eq!(observations, [Observation::SubjectLength { length: 5 }]);
+    }
+
+    #[test]
+    fn multiple_commits() {
+        let repo = repository::Builder::new()
+            .commit("Hi")
+            .commit("Hello world")
+            .build()
+            .unwrap();
+        let observations = observe_all(&repo, SubjectLengthObserver);
+        assert_eq!(
+            observations,
+            [
+                Observation::SubjectLength { length: 2 },
+                Observation::SubjectLength { length: 11 },
+            ]
+        );
+    }
+}