Refactor shebang handling (#833)

src/platform.rs

@@ -7,8 +7,7 @@ impl PlatformInterface for Platform {
   fn make_shebang_command(
     path: &Path,
     working_directory: &Path,
-    _command: &str,
+    _shebang: Shebang,
-    _argument: Option<&str>,
   ) -> Result<Command, OutputError> {
     // shebang scripts can be executed directly on unix
     let mut cmd = Command::new(path);
@@ -50,30 +49,29 @@ impl PlatformInterface for Platform {
   fn make_shebang_command(
     path: &Path,
     working_directory: &Path,
-    command: &str,
+    shebang: Shebang,
-    argument: Option<&str>,
   ) -> Result<Command, OutputError> {
     use std::borrow::Cow;
 
     // If the path contains forward slashes…
-    let command = if command.contains('/') {
+    let command = if shebang.interpreter.contains('/') {
       // …translate path to the interpreter from unix style to windows style.
       let mut cygpath = Command::new("cygpath");
       cygpath.current_dir(working_directory);
       cygpath.arg("--windows");
-      cygpath.arg(command);
+      cygpath.arg(shebang.interpreter);
 
       Cow::Owned(output(cygpath)?)
     } else {
       // …otherwise use it as-is.
-      Cow::Borrowed(command)
+      Cow::Borrowed(shebang.interpreter)
     };
 
     let mut cmd = Command::new(command.as_ref());
 
     cmd.current_dir(working_directory);
 
-    if let Some(argument) = argument {
+    if let Some(argument) = shebang.argument {
       cmd.arg(argument);
     }
 

src/platform_interface.rs

@@ -6,8 +6,7 @@ pub(crate) trait PlatformInterface {
   fn make_shebang_command(
     path: &Path,
     working_directory: &Path,
-    command: &str,
+    shebang: Shebang,
-    argument: Option<&str>,
   ) -> Result<Command, OutputError>;
 
   /// Set the execute permission on the file pointed to by `path`

src/recipe.rs

@@ -113,12 +113,10 @@ impl<'src, D> Recipe<'src, D> {
           message: "evaluated_lines was empty".to_owned(),
         })?;
 
-      let Shebang {
+      let shebang = Shebang::new(shebang_line).ok_or_else(|| RuntimeError::Internal {
-        interpreter,
-        argument,
-      } = Shebang::new(shebang_line).ok_or_else(|| RuntimeError::Internal {
         message: format!("bad shebang line: {}", shebang_line),
       })?;
+
       let tmp = tempfile::Builder::new()
         .prefix("just")
         .tempdir()
@@ -127,26 +125,22 @@ impl<'src, D> Recipe<'src, D> {
           io_error: error,
         })?;
       let mut path = tmp.path().to_path_buf();
-      let suffix = if interpreter.ends_with("cmd") || interpreter.ends_with("cmd.exe") {
+
-        ".bat"
+      path.push(shebang.script_filename(self.name()));
-      } else if interpreter.ends_with("powershell") || interpreter.ends_with("powershell.exe") {
+
-        ".ps1"
-      } else {
-        ""
-      };
-      path.push(format!("{}{}", self.name(), suffix));
       {
         let mut f = fs::File::create(&path).map_err(|error| RuntimeError::TmpdirIoError {
           recipe:   self.name(),
           io_error: error,
         })?;
         let mut text = String::new();
-        // add the shebang
+
-        if interpreter.ends_with("cmd") || interpreter.ends_with("cmd.exe") {
+        if shebang.include_shebang_line() {
-          text += "\n";
-        } else {
           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
@@ -176,16 +170,13 @@ impl<'src, D> Recipe<'src, D> {
       })?;
 
       // create a command to run the script
-      let mut command = Platform::make_shebang_command(
+      let mut command =
-        &path,
+        Platform::make_shebang_command(&path, &context.search.working_directory, shebang).map_err(
-        &context.search.working_directory,
+          |output_error| RuntimeError::Cygpath {
-        interpreter,
-        argument,
-      )
-      .map_err(|output_error| RuntimeError::Cygpath {
             recipe: self.name(),
             output_error,
-      })?;
+          },
+        )?;
 
       if context.settings.positional_arguments {
         command.args(positional);
@@ -210,8 +201,8 @@ impl<'src, D> Recipe<'src, D> {
         Err(io_error) => {
           return Err(RuntimeError::Shebang {
             recipe: self.name(),
-            command: interpreter.to_owned(),
+            command: shebang.interpreter.to_owned(),
-            argument: argument.map(String::from),
+            argument: shebang.argument.map(String::from),
             io_error,
           });
         },

src/shebang.rs

@@ -1,3 +1,4 @@
+#[derive(Copy, Clone)]
 pub(crate) struct Shebang<'line> {
   pub(crate) interpreter: &'line str,
   pub(crate) argument:    Option<&'line str>,
@@ -28,6 +29,26 @@ impl<'line> Shebang<'line> {
       argument,
     })
   }
+
+  fn interpreter_filename(&self) -> &str {
+    self
+      .interpreter
+      .rsplit_once(|c| matches!(c, '/' | '\\'))
+      .map(|(_path, filename)| filename)
+      .unwrap_or(self.interpreter)
+  }
+
+  pub(crate) fn script_filename(&self, recipe: &str) -> String {
+    match self.interpreter_filename() {
+      "cmd" | "cmd.exe" => format!("{}.bat", recipe),
+      "powershell" | "powershell.exe" => format!("{}.ps1", recipe),
+      _ => recipe.to_owned(),
+    }
+  }
+
+  pub(crate) fn include_shebang_line(&self) -> bool {
+    !matches!(self.interpreter_filename(), "cmd" | "cmd.exe")
+  }
 }
 
 #[cfg(test)]
@@ -93,4 +114,78 @@ mod tests {
     );
     check("#  /usr/bin/env python \t-x\t", None);
   }
+
+  #[test]
+  fn interpreter_filename_with_forward_slash() {
+    assert_eq!(
+      Shebang::new("#!/foo/bar/baz")
+        .unwrap()
+        .interpreter_filename(),
+      "baz"
+    );
+  }
+
+  #[test]
+  fn interpreter_filename_with_backslash() {
+    assert_eq!(
+      Shebang::new("#!\\foo\\bar\\baz")
+        .unwrap()
+        .interpreter_filename(),
+      "baz"
+    );
+  }
+
+  #[test]
+  fn powershell_script_filename() {
+    assert_eq!(
+      Shebang::new("#!powershell").unwrap().script_filename("foo"),
+      "foo.ps1"
+    );
+  }
+
+  #[test]
+  fn powershell_exe_script_filename() {
+    assert_eq!(
+      Shebang::new("#!powershell.exe")
+        .unwrap()
+        .script_filename("foo"),
+      "foo.ps1"
+    );
+  }
+
+  #[test]
+  fn cmd_script_filename() {
+    assert_eq!(
+      Shebang::new("#!cmd").unwrap().script_filename("foo"),
+      "foo.bat"
+    );
+  }
+
+  #[test]
+  fn cmd_exe_script_filename() {
+    assert_eq!(
+      Shebang::new("#!cmd.exe").unwrap().script_filename("foo"),
+      "foo.bat"
+    );
+  }
+
+  #[test]
+  fn plain_script_filename() {
+    assert_eq!(Shebang::new("#!bar").unwrap().script_filename("foo"), "foo");
+  }
+
+  #[test]
+  fn dont_include_shebang_line_cmd() {
+    assert!(!Shebang::new("#!cmd").unwrap().include_shebang_line());
+  }
+
+  #[test]
+  fn dont_include_shebang_line_cmd_exe() {
+    assert!(!Shebang::new("#!cmd.exe /C").unwrap().include_shebang_line());
+  }
+
+  #[test]
+  fn include_shebang_line_other() {
+    assert!(Shebang::new("#!foo -c").unwrap().include_shebang_line());
+  }
 }