From 4a4c669db9454032e17d1304a5f81f1bb47b79a6 Mon Sep 17 00:00:00 2001
From: Casey Rodarmor <casey@rodarmor.com>
Date: Wed, 20 Jul 2022 18:46:52 -0700
Subject: [PATCH] Split Recipe::run into Recipe::{run_shebang,run_linewise}
 (#1270)

---
 justfile          |   7 +
 src/lib.rs        |   1 +
 src/recipe.rs     | 424 ++++++++++++++++++++++++----------------------
 src/subcommand.rs |   1 -
 4 files changed, 231 insertions(+), 202 deletions(-)

diff --git a/justfile b/justfile
index f3bf567..c9c6708 100755
--- a/justfile
+++ b/justfile
@@ -17,6 +17,13 @@ export JUST_LOG := log
 test:
   cargo test
 
+ci: build-book
+  cargo test --all
+  cargo clippy --all --all-targets
+  cargo fmt --all -- --check
+  ./bin/forbid
+  cargo update --locked --package just
+
 fuzz:
   cargo +nightly fuzz run fuzz-compiler
 
diff --git a/src/lib.rs b/src/lib.rs
index 284b827..667e04b 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -11,6 +11,7 @@
   clippy::shadow_unrelated,
   clippy::struct_excessive_bools,
   clippy::too_many_lines,
+  clippy::type_repetition_in_bounds,
   clippy::wildcard_imports
 )]
 
diff --git a/src/recipe.rs b/src/recipe.rs
index 0b4ee36..63787a3 100644
--- a/src/recipe.rs
+++ b/src/recipe.rs
@@ -85,225 +85,247 @@ impl<'src, D> Recipe<'src, D> {
       );
     }
 
-    let mut evaluator =
+    let evaluator =
       Evaluator::recipe_evaluator(context.config, dotenv, &scope, context.settings, search);
 
     if self.shebang {
-      let mut evaluated_lines = vec![];
-      for line in &self.body {
-        evaluated_lines.push(evaluator.evaluate_line(line, false)?);
-      }
+      self.run_shebang(context, dotenv, &scope, positional, config, evaluator)
+    } else {
+      self.run_linewise(context, dotenv, &scope, positional, config, evaluator)
+    }
+  }
 
-      if config.verbosity.loud() && (config.dry_run || self.quiet) {
-        for line in &evaluated_lines {
-          eprintln!("{}", line);
-        }
-      }
-
-      if config.dry_run {
+  pub(crate) fn run_linewise<'run>(
+    &self,
+    context: &RecipeContext<'src, 'run>,
+    dotenv: &BTreeMap<String, String>,
+    scope: &Scope<'src, 'run>,
+    positional: &[String],
+    config: &Config,
+    mut evaluator: Evaluator<'src, 'run>,
+  ) -> RunResult<'src, ()> {
+    let mut lines = self.body.iter().peekable();
+    let mut line_number = self.line_number() + 1;
+    loop {
+      if lines.peek().is_none() {
         return Ok(());
       }
-
-      let shebang_line = evaluated_lines.first().ok_or_else(|| Error::Internal {
-        message: "evaluated_lines was empty".to_owned(),
-      })?;
-
-      let shebang = Shebang::new(shebang_line).ok_or_else(|| Error::Internal {
-        message: format!("bad shebang line: {}", shebang_line),
-      })?;
-
-      let tmp = tempfile::Builder::new()
-        .prefix("just")
-        .tempdir()
-        .map_err(|error| Error::TmpdirIo {
-          recipe: self.name(),
-          io_error: error,
-        })?;
-      let mut path = tmp.path().to_path_buf();
-
-      path.push(shebang.script_filename(self.name()));
-
-      {
-        let mut f = fs::File::create(&path).map_err(|error| Error::TmpdirIo {
-          recipe: self.name(),
-          io_error: error,
-        })?;
-        let mut text = String::new();
-
-        if shebang.include_shebang_line() {
-          text += &evaluated_lines[0];
-        } else {
-          text += "\n";
-        }
-
-        text += "\n";
-        // add blank lines so that lines in the generated script have the same line
-        // number as the corresponding lines in the justfile
-        for _ in 1..(self.line_number() + 2) {
-          text += "\n";
-        }
-        for line in &evaluated_lines[1..] {
-          text += line;
-          text += "\n";
-        }
-
-        if config.verbosity.grandiloquent() {
-          eprintln!("{}", config.color.doc().stderr().paint(&text));
-        }
-
-        f.write_all(text.as_bytes())
-          .map_err(|error| Error::TmpdirIo {
-            recipe: self.name(),
-            io_error: error,
-          })?;
-      }
-
-      // make the script executable
-      Platform::set_execute_permission(&path).map_err(|error| Error::TmpdirIo {
-        recipe: self.name(),
-        io_error: error,
-      })?;
-
-      // create a command to run the script
-      let mut command =
-        Platform::make_shebang_command(&path, &context.search.working_directory, shebang).map_err(
-          |output_error| Error::Cygpath {
-            recipe: self.name(),
-            output_error,
-          },
-        )?;
-
-      if context.settings.positional_arguments {
-        command.args(positional);
-      }
-
-      command.export(context.settings, dotenv, &scope);
-
-      // run it!
-      match InterruptHandler::guard(|| command.status()) {
-        Ok(exit_status) => {
-          if let Some(code) = exit_status.code() {
-            if code != 0 {
-              return Err(Error::Code {
-                recipe: self.name(),
-                line_number: None,
-                code,
-              });
-            }
-          } else {
-            return Err(error_from_signal(self.name(), None, exit_status));
-          }
-        }
-        Err(io_error) => {
-          return Err(Error::Shebang {
-            recipe: self.name(),
-            command: shebang.interpreter.to_owned(),
-            argument: shebang.argument.map(String::from),
-            io_error,
-          });
-        }
-      };
-    } else {
-      let mut lines = self.body.iter().peekable();
-      let mut line_number = self.line_number() + 1;
+      let mut evaluated = String::new();
+      let mut continued = false;
+      let quiet_command = lines.peek().map_or(false, |line| line.is_quiet());
+      let infallible_command = lines.peek().map_or(false, |line| line.is_infallible());
       loop {
         if lines.peek().is_none() {
           break;
         }
-        let mut evaluated = String::new();
-        let mut continued = false;
-        let quiet_command = lines.peek().map_or(false, |line| line.is_quiet());
-        let infallible_command = lines.peek().map_or(false, |line| line.is_infallible());
-        loop {
-          if lines.peek().is_none() {
-            break;
-          }
-          let line = lines.next().unwrap();
-          line_number += 1;
-          evaluated += &evaluator.evaluate_line(line, continued)?;
-          if line.is_continuation() {
-            continued = true;
-            evaluated.pop();
-          } else {
-            break;
-          }
+        let line = lines.next().unwrap();
+        line_number += 1;
+        evaluated += &evaluator.evaluate_line(line, continued)?;
+        if line.is_continuation() {
+          continued = true;
+          evaluated.pop();
+        } else {
+          break;
         }
-        let mut command = evaluated.as_str();
+      }
+      let mut command = evaluated.as_str();
 
-        if quiet_command {
-          command = &command[1..];
-        }
+      if quiet_command {
+        command = &command[1..];
+      }
 
-        if infallible_command {
-          command = &command[1..];
-        }
+      if infallible_command {
+        command = &command[1..];
+      }
 
-        if command.is_empty() {
-          continue;
-        }
+      if command.is_empty() {
+        continue;
+      }
 
-        if config.dry_run
-          || config.verbosity.loquacious()
-          || !((quiet_command ^ self.quiet) || config.verbosity.quiet())
-        {
-          let color = if config.highlight {
-            config.color.command()
-          } else {
-            config.color
-          };
-          eprintln!("{}", color.stderr().paint(command));
-        }
-
-        if config.dry_run {
-          continue;
-        }
-
-        let mut cmd = context.settings.shell_command(config);
-
-        cmd.current_dir(&context.search.working_directory);
-
-        cmd.arg(command);
-
-        if context.settings.positional_arguments {
-          cmd.arg(self.name.lexeme());
-          cmd.args(positional);
-        }
-
-        if config.verbosity.quiet() {
-          cmd.stderr(Stdio::null());
-          cmd.stdout(Stdio::null());
-        }
-
-        cmd.export(context.settings, dotenv, &scope);
-
-        match InterruptHandler::guard(|| cmd.status()) {
-          Ok(exit_status) => {
-            if let Some(code) = exit_status.code() {
-              if code != 0 && !infallible_command {
-                return Err(Error::Code {
-                  recipe: self.name(),
-                  line_number: Some(line_number),
-                  code,
-                });
-              }
-            } else {
-              return Err(error_from_signal(
-                self.name(),
-                Some(line_number),
-                exit_status,
-              ));
-            }
-          }
-          Err(io_error) => {
-            return Err(Error::Io {
-              recipe: self.name(),
-              io_error,
-            });
-          }
+      if config.dry_run
+        || config.verbosity.loquacious()
+        || !((quiet_command ^ self.quiet) || config.verbosity.quiet())
+      {
+        let color = if config.highlight {
+          config.color.command()
+        } else {
+          config.color
         };
+        eprintln!("{}", color.stderr().paint(command));
+      }
+
+      if config.dry_run {
+        continue;
+      }
+
+      let mut cmd = context.settings.shell_command(config);
+
+      cmd.current_dir(&context.search.working_directory);
+
+      cmd.arg(command);
+
+      if context.settings.positional_arguments {
+        cmd.arg(self.name.lexeme());
+        cmd.args(positional);
+      }
+
+      if config.verbosity.quiet() {
+        cmd.stderr(Stdio::null());
+        cmd.stdout(Stdio::null());
+      }
+
+      cmd.export(context.settings, dotenv, scope);
+
+      match InterruptHandler::guard(|| cmd.status()) {
+        Ok(exit_status) => {
+          if let Some(code) = exit_status.code() {
+            if code != 0 && !infallible_command {
+              return Err(Error::Code {
+                recipe: self.name(),
+                line_number: Some(line_number),
+                code,
+              });
+            }
+          } else {
+            return Err(error_from_signal(
+              self.name(),
+              Some(line_number),
+              exit_status,
+            ));
+          }
+        }
+        Err(io_error) => {
+          return Err(Error::Io {
+            recipe: self.name(),
+            io_error,
+          });
+        }
+      };
+    }
+  }
+
+  pub(crate) fn run_shebang<'run>(
+    &self,
+    context: &RecipeContext<'src, 'run>,
+    dotenv: &BTreeMap<String, String>,
+    scope: &Scope<'src, 'run>,
+    positional: &[String],
+    config: &Config,
+    mut evaluator: Evaluator<'src, 'run>,
+  ) -> RunResult<'src, ()> {
+    let mut evaluated_lines = vec![];
+    for line in &self.body {
+      evaluated_lines.push(evaluator.evaluate_line(line, false)?);
+    }
+
+    if config.verbosity.loud() && (config.dry_run || self.quiet) {
+      for line in &evaluated_lines {
+        eprintln!("{}", line);
       }
     }
-    Ok(())
+
+    if config.dry_run {
+      return Ok(());
+    }
+
+    let shebang_line = evaluated_lines.first().ok_or_else(|| Error::Internal {
+      message: "evaluated_lines was empty".to_owned(),
+    })?;
+
+    let shebang = Shebang::new(shebang_line).ok_or_else(|| Error::Internal {
+      message: format!("bad shebang line: {}", shebang_line),
+    })?;
+
+    let tmp = tempfile::Builder::new()
+      .prefix("just")
+      .tempdir()
+      .map_err(|error| Error::TmpdirIo {
+        recipe: self.name(),
+        io_error: error,
+      })?;
+    let mut path = tmp.path().to_path_buf();
+
+    path.push(shebang.script_filename(self.name()));
+
+    {
+      let mut f = fs::File::create(&path).map_err(|error| Error::TmpdirIo {
+        recipe: self.name(),
+        io_error: error,
+      })?;
+      let mut text = String::new();
+
+      if shebang.include_shebang_line() {
+        text += &evaluated_lines[0];
+      } else {
+        text += "\n";
+      }
+
+      text += "\n";
+      // add blank lines so that lines in the generated script have the same line
+      // number as the corresponding lines in the justfile
+      for _ in 1..(self.line_number() + 2) {
+        text += "\n";
+      }
+      for line in &evaluated_lines[1..] {
+        text += line;
+        text += "\n";
+      }
+
+      if config.verbosity.grandiloquent() {
+        eprintln!("{}", config.color.doc().stderr().paint(&text));
+      }
+
+      f.write_all(text.as_bytes())
+        .map_err(|error| Error::TmpdirIo {
+          recipe: self.name(),
+          io_error: error,
+        })?;
+    }
+
+    // make the script executable
+    Platform::set_execute_permission(&path).map_err(|error| Error::TmpdirIo {
+      recipe: self.name(),
+      io_error: error,
+    })?;
+
+    // create a command to run the script
+    let mut command =
+      Platform::make_shebang_command(&path, &context.search.working_directory, shebang).map_err(
+        |output_error| Error::Cygpath {
+          recipe: self.name(),
+          output_error,
+        },
+      )?;
+
+    if context.settings.positional_arguments {
+      command.args(positional);
+    }
+
+    command.export(context.settings, dotenv, scope);
+
+    // run it!
+    match InterruptHandler::guard(|| command.status()) {
+      Ok(exit_status) => exit_status.code().map_or_else(
+        || Err(error_from_signal(self.name(), None, exit_status)),
+        |code| {
+          if code == 0 {
+            Ok(())
+          } else {
+            Err(Error::Code {
+              recipe: self.name(),
+              line_number: None,
+              code,
+            })
+          }
+        },
+      ),
+      Err(io_error) => Err(Error::Shebang {
+        recipe: self.name(),
+        command: shebang.interpreter.to_owned(),
+        argument: shebang.argument.map(String::from),
+        io_error,
+      }),
+    }
   }
 }
 
diff --git a/src/subcommand.rs b/src/subcommand.rs
index ea7e9e5..1a56e9e 100644
--- a/src/subcommand.rs
+++ b/src/subcommand.rs
@@ -254,7 +254,6 @@ impl Subcommand {
     let stdout = String::from_utf8_lossy(&output.stdout);
 
     let recipes = stdout
-      .trim()
       .split_whitespace()
       .map(str::to_owned)
       .collect::<Vec<String>>();