refactor: drop builder pattern for Decision

It was a little easier for testing, but not worth having for
production code.

Signed-off-by: Lars Wirzenius <liw@liw.fi>
Sponsored-by: author

1 files changed, 97 insertions, 105 deletions

diff --git a/src/run.rs b/src/run.rs
index 5426f18..d0a43a3 100644
--- a/src/run.rs
+++ b/src/run.rs
@@ -150,50 +150,52 @@ fn should_run(
     let state = State::from_file(statedir, name)?;
     if let Some(latest_commit) = state.latest_commit() {
         debug!("latest commit: {:?}", latest_commit);
-        decision = decision.latest_commit(latest_commit);
+        decision.latest_commit(latest_commit);
     } else {
         debug!("no latest commit stored");
+        // No need to set latest commit to anything, the default is OK.
     }
 
     let is_git = is_git(project.source());
     if is_git {
         debug!("is a git repository");
-        decision = decision.is_git();
+        decision.is_git();
     } else {
         debug!("is not a git repository");
-        decision = decision.is_not_git();
+        decision.is_not_git();
     }
 
     if !git_is_clean(project.source()) {
         debug!("git repository is clean");
-        decision = decision.is_clean();
+        decision.is_clean();
     } else {
         debug!("git repository is dirty");
-        decision = decision.is_dirty();
+        decision.is_dirty();
     }
 
     if is_git {
         let head = git_head(project.source())?;
         debug!("current (HEAD) commit: {head}");
-        decision = decision.current_commit(&head);
+        decision.current_commit(&head);
     } else {
         debug!("no current commit due to not git repository");
+        // No need to set current commit to anything, the default is OK.
     }
 
     if run.dry_run() {
         debug!("dry run requested");
-        decision = decision.dry_run();
+        decision.dry_run();
     } else {
         debug!("no dry run requested");
-        decision = decision.no_dry_run();
+        decision.no_dry_run();
     }
 
     if run.force() {
         debug!("forced run requested");
-        decision = decision.force();
+        decision.force();
     } else {
         debug!("no forced run requested");
-        decision = decision.no_force();
+        decision.no_force();
     }
 
     let do_run = decision.should_run() == ShouldRun::Run;
@@ -365,54 +367,44 @@ struct Decision {
 }
 
 impl Decision {
-    fn dry_run(mut self) -> Self {
+    fn dry_run(&mut self) {
         self.dry_run = Some(true);
-        self
     }
 
-    fn no_dry_run(mut self) -> Self {
+    fn no_dry_run(&mut self) {
         self.dry_run = Some(false);
-        self
     }
 
-    fn force(mut self) -> Self {
+    fn force(&mut self) {
         self.force_run = Some(true);
-        self
     }
 
-    fn no_force(mut self) -> Self {
+    fn no_force(&mut self) {
         self.force_run = Some(false);
-        self
     }
 
-    fn is_git(mut self) -> Self {
+    fn is_git(&mut self) {
         self.is_git = Some(true);
-        self
     }
 
-    fn is_not_git(mut self) -> Self {
+    fn is_not_git(&mut self) {
         self.is_git = Some(false);
-        self
     }
 
-    fn latest_commit(mut self, commit: &str) -> Self {
+    fn latest_commit(&mut self, commit: &str) {
         self.latest_commit = Some(commit.into());
-        self
     }
 
-    fn current_commit(mut self, commit: &str) -> Self {
+    fn current_commit(&mut self, commit: &str) {
         self.current_commit = Some(commit.into());
-        self
     }
 
-    fn is_clean(mut self) -> Self {
+    fn is_clean(&mut self) {
         self.source_is_dirty = Some(false);
-        self
     }
 
-    fn is_dirty(mut self) -> Self {
+    fn is_dirty(&mut self) {
         self.source_is_dirty = Some(true);
-        self
     }
 
     fn should_run(&self) -> ShouldRun {
@@ -463,131 +455,131 @@ mod test_run_decision {
 
     #[test]
     fn is_not_git() {
-        let d = Decision::default()
-            .no_dry_run()
-            .no_force()
-            .is_not_git()
-            .is_clean();
+        let mut d = Decision::default();
+        d.no_dry_run();
+        d.no_force();
+        d.is_not_git();
+        d.is_clean();
         assert_eq!(d.should_run(), ShouldRun::Run);
     }
 
     #[test]
     fn unchanged() {
-        let d = Decision::default()
-            .no_dry_run()
-            .no_force()
-            .is_git()
-            .is_clean()
-            .latest_commit("abcd")
-            .current_commit("abcd");
+        let mut d = Decision::default();
+        d.no_dry_run();
+        d.no_force();
+        d.is_git();
+        d.is_clean();
+        d.latest_commit("abcd");
+        d.current_commit("abcd");
         assert_eq!(d.should_run(), ShouldRun::DontRun);
     }
 
     #[test]
     fn unchanged_with_force() {
-        let d = Decision::default()
-            .no_dry_run()
-            .force()
-            .is_git()
-            .is_clean()
-            .latest_commit("abcd")
-            .current_commit("abcd");
+        let mut d = Decision::default();
+        d.no_dry_run();
+        d.force();
+        d.is_git();
+        d.is_clean();
+        d.latest_commit("abcd");
+        d.current_commit("abcd");
         assert_eq!(d.should_run(), ShouldRun::Run);
     }
 
     #[test]
     fn unchanged_commit_but_dirty() {
-        let d = Decision::default()
-            .no_dry_run()
-            .no_force()
-            .is_git()
-            .is_dirty()
-            .latest_commit("abcd")
-            .current_commit("abcd");
+        let mut d = Decision::default();
+        d.no_dry_run();
+        d.no_force();
+        d.is_git();
+        d.is_dirty();
+        d.latest_commit("abcd");
+        d.current_commit("abcd");
         assert_eq!(d.should_run(), ShouldRun::Run);
     }
 
     #[test]
     fn commit_changed() {
-        let d = Decision::default()
-            .no_dry_run()
-            .no_force()
-            .is_git()
-            .is_clean()
-            .latest_commit("abcd")
-            .current_commit("efgh");
+        let mut d = Decision::default();
+        d.no_dry_run();
+        d.no_force();
+        d.is_git();
+        d.is_clean();
+        d.latest_commit("abcd");
+        d.current_commit("efgh");
         assert_eq!(d.should_run(), ShouldRun::Run);
     }
 
     #[test]
     fn dry_run_for_unchanged() {
-        let d = Decision::default()
-            .dry_run()
-            .no_force()
-            .is_git()
-            .is_clean()
-            .latest_commit("abcd")
-            .current_commit("abcd");
+        let mut d = Decision::default();
+        d.dry_run();
+        d.no_force();
+        d.is_git();
+        d.is_clean();
+        d.latest_commit("abcd");
+        d.current_commit("abcd");
         assert_eq!(d.should_run(), ShouldRun::DontRun);
     }
 
     #[test]
     fn dry_run_for_unchanged_but_dirty() {
-        let d = Decision::default()
-            .dry_run()
-            .no_force()
-            .is_git()
-            .is_dirty()
-            .latest_commit("abcd")
-            .current_commit("efgh");
+        let mut d = Decision::default();
+        d.dry_run();
+        d.no_force();
+        d.is_git();
+        d.is_dirty();
+        d.latest_commit("abcd");
+        d.current_commit("efgh");
         assert_eq!(d.should_run(), ShouldRun::DontRun);
     }
 
     #[test]
     fn dry_run_for_commit_changed() {
-        let d = Decision::default()
-            .dry_run()
-            .no_force()
-            .is_git()
-            .is_clean()
-            .latest_commit("abcd")
-            .current_commit("efgh");
+        let mut d = Decision::default();
+        d.dry_run();
+        d.no_force();
+        d.is_git();
+        d.is_clean();
+        d.latest_commit("abcd");
+        d.current_commit("efgh");
         assert_eq!(d.should_run(), ShouldRun::DontRun);
     }
 
     #[test]
     fn dry_run_for_unchanged_with_force() {
-        let d = Decision::default()
-            .dry_run()
-            .no_force()
-            .is_git()
-            .is_clean()
-            .latest_commit("abcd")
-            .current_commit("abcd");
+        let mut d = Decision::default();
+        d.dry_run();
+        d.no_force();
+        d.is_git();
+        d.is_clean();
+        d.latest_commit("abcd");
+        d.current_commit("abcd");
         assert_eq!(d.should_run(), ShouldRun::DontRun);
     }
 
     #[test]
     fn dry_run_for_unchanged_but_dirty_with_force() {
-        let d = Decision::default()
-            .dry_run()
-            .no_force()
-            .is_git()
-            .is_dirty()
-            .latest_commit("abcd")
-            .current_commit("efgh");
+        let mut d = Decision::default();
+        d.dry_run();
+        d.no_force();
+        d.is_git();
+        d.is_dirty();
+        d.latest_commit("abcd");
+        d.current_commit("efgh");
         assert_eq!(d.should_run(), ShouldRun::DontRun);
     }
 
     #[test]
     fn dry_run_for_commit_changed_with_force() {
-        let d = Decision::default()
-            .dry_run()
-            .no_force()
-            .is_git()
-            .is_clean()
-            .latest_commit("abcd")
-            .current_commit("efgh");
+        let mut d = Decision::default();
+        d.dry_run();
+        d.no_force();
+        d.is_git();
+        d.is_clean();
+        d.latest_commit("abcd");
+        d.current_commit("efgh");
         assert_eq!(d.should_run(), ShouldRun::DontRun);
     }
 }