ronantakizawa/codereview-bench huggingface dataset:
Repository: casey/just
File: src/settings.rs (Rust)

Code before:
use crate::common::*;

pub(crate) const DEFAULT_SHELL: &str = "sh";
pub(crate) const DEFAULT_SHELL_ARG: &[&str] = &["-cu"];
pub(crate) const WINDOWS_DEFAULT_SHELL: &str = "powershell.exe";
pub(crate) const WINDOWS_DEFAULT_SHELL_ARG: &[&str] = &["-NoLogo", "-Command"];

#[derive(Debug, PartialEq, Serialize)]
pub(crate) struct Settings<'src> {
  pub(crate) dotenv_load: Option<bool>,
  pub(crate) export: bool,
  pub(crate) positional_arguments: bool,
  pub(crate) shell: Option<setting::Shell<'src>>,
  pub(crate) windows_powershell: bool,
}

impl<'src> Settings<'src> {
  pub(crate) fn new() -> Settings<'src> {
    Settings {
      dotenv_load: None,
      export: false,
      positional_arguments: false,
      shell: None,
      windows_powershell: false,
    }
  }

  pub(crate) fn shell_command(&self, config: &Config) -> Command {
    let mut cmd = Command::new(self.shell_binary(config));

    cmd.args(self.shell_arguments(config));

Review comment (suggestion):
```suggestion
pub(crate) const WINDOWS_DEFAULT_SHELL_ARGS: &[&str] = &["-NoLogo", "-Command"];
```

Code after:
use crate::common::*;

pub(crate) const DEFAULT_SHELL: &str = "sh";
pub(crate) const DEFAULT_SHELL_ARGS: &[&str] = &["-cu"];
pub(crate) const WINDOWS_POWERSHELL_SHELL: &str = "powershell.exe";
pub(crate) const WINDOWS_POWERSHELL_ARGS: &[&str] = &["-NoLogo", "-Command"];

#[derive(Debug, PartialEq, Serialize)]
pub(crate) struct Settings<'src> {
  pub(crate) dotenv_load: Option<bool>,
  pub(crate) export: bool,
  pub(crate) positional_arguments: bool,
  pub(crate) shell: Option<setting::Shell<'src>>,
  pub(crate) windows_powershell: bool,
}

impl<'src> Settings<'src> {
  pub(crate) fn new() -> Settings<'src> {
    Settings {
      dotenv_load: None,
      export: false,
      positional_arguments: false,
      shell: None,
      windows_powershell: false,
    }
  }

  pub(crate) fn shell_command(&self, config: &Config) -> Command {
    let mut cmd = Command::new(self.shell_binary(config));

    cmd.args(self.shell_arguments(config));
Repository: casey/just
File: src/settings.rs (Rust)

Code before:
}

impl<'src> Settings<'src> {
  pub(crate) fn new() -> Settings<'src> {
    Settings {
      dotenv_load: None,
      export: false,
      positional_arguments: false,
      shell: None,
      windows_powershell: false,
    }
  }

  pub(crate) fn shell_command(&self, config: &Config) -> Command {
    let mut cmd = Command::new(self.shell_binary(config));

    cmd.args(self.shell_arguments(config));

    cmd
  }

  pub(crate) fn shell_binary<'a>(&'a self, config: &'a Config) -> &'a str {
    // Order of precedence:
    // 1) CLI argument (config)
    // 2) Settings in justfile (settings)
    // 3) Default/PowerShell
    let shell_or_args_present = config.shell_present | config.shell_args_present;

    if let (Some(shell), false) = (&self.shell, shell_or_args_present) {
      shell.command.cooked.as_ref()
    } else if config.shell_present {
      &config.shell
    } else {
      // Default value for shell
      if cfg!(windows) && self.windows_powershell {
        WINDOWS_DEFAULT_SHELL
      } else {
        DEFAULT_SHELL
      }
    }
  }

  pub(crate) fn shell_arguments<'a>(&'a self, config: &'a Config) -> Vec<&'a str> {
    // Order of precedence like in fn shell_binary
    let shell_or_args_present = config.shell_present | config.shell_args_present;

    if let (Some(shell), false) = (&self.shell, shell_or_args_present) {
      shell
        .arguments
        .iter()
        .map(|argument| argument.cooked.as_ref())

Review comment (refactor):
I think this is mostly evident from reading the code, so I'd remove this comment.

```suggestion
```

Code after:
}

impl<'src> Settings<'src> {
  pub(crate) fn new() -> Settings<'src> {
    Settings {
      dotenv_load: None,
      export: false,
      positional_arguments: false,
      shell: None,
      windows_powershell: false,
    }
  }

  pub(crate) fn shell_command(&self, config: &Config) -> Command {
    let mut cmd = Command::new(self.shell_binary(config));

    cmd.args(self.shell_arguments(config));

    cmd
  }

  pub(crate) fn shell_binary<'a>(&'a self, config: &'a Config) -> &'a str {
    let shell_or_args_present = config.shell.is_some() || config.shell_args.is_some();

    if let (Some(shell), false) = (&self.shell, shell_or_args_present) {
      shell.command.cooked.as_ref()
    } else if let Some(shell) = &config.shell {
      shell
    } else if cfg!(windows) && self.windows_powershell {
      WINDOWS_POWERSHELL_SHELL
    } else {
      DEFAULT_SHELL
    }
  }

  pub(crate) fn shell_arguments<'a>(&'a self, config: &'a Config) -> Vec<&'a str> {
    let shell_or_args_present = config.shell.is_some() || config.shell_args.is_some();

    if let (Some(shell), false) = (&self.shell, shell_or_args_present) {
      shell
        .arguments
        .iter()
        .map(|argument| argument.cooked.as_ref())
        .collect()
    } else if let Some(shell_args) = &config.shell_args {
      shell_args.iter().map(String::as_ref).collect()
    } else if cfg!(windows) && self.windows_powershell {
      WINDOWS_POWERSHELL_ARGS.to_vec()
    } else {
      DEFAULT_SHELL_ARGS.to_vec()
    }
Repository: casey/just
File: src/settings.rs (Rust)

Code before:
use crate::common::*;

pub(crate) const DEFAULT_SHELL: &str = "sh";
pub(crate) const DEFAULT_SHELL_ARG: &[&str] = &["-cu"];
pub(crate) const WINDOWS_DEFAULT_SHELL: &str = "powershell.exe";
pub(crate) const WINDOWS_DEFAULT_SHELL_ARGS: &[&str] = &["-NoLogo", "-Command"];

#[derive(Debug, PartialEq, Serialize)]
pub(crate) struct Settings<'src> {
  pub(crate) dotenv_load: Option<bool>,
  pub(crate) export: bool,
  pub(crate) positional_arguments: bool,
  pub(crate) shell: Option<setting::Shell<'src>>,
  pub(crate) windows_powershell: bool,
}

impl<'src> Settings<'src> {
  pub(crate) fn new() -> Settings<'src> {
    Settings {
      dotenv_load: None,
      export: false,
      positional_arguments: false,
      shell: None,
      windows_powershell: false,
    }
  }

  pub(crate) fn shell_command(&self, config: &Config) -> Command {
    let mut cmd = Command::new(self.shell_binary(config));

    cmd.args(self.shell_arguments(config));

Review comment (suggestion):
```suggestion
pub(crate) const WINDOWS_POWERSHELL_SHELL: &str = "powershell.exe";
pub(crate) const WINDOWS_POWERSHELL_ARGS: &[&str] = &["-NoLogo", "-Command"];
```

Code after:
use crate::common::*;

pub(crate) const DEFAULT_SHELL: &str = "sh";
pub(crate) const DEFAULT_SHELL_ARGS: &[&str] = &["-cu"];
pub(crate) const WINDOWS_POWERSHELL_SHELL: &str = "powershell.exe";
pub(crate) const WINDOWS_POWERSHELL_ARGS: &[&str] = &["-NoLogo", "-Command"];

#[derive(Debug, PartialEq, Serialize)]
pub(crate) struct Settings<'src> {
  pub(crate) dotenv_load: Option<bool>,
  pub(crate) export: bool,
  pub(crate) positional_arguments: bool,
  pub(crate) shell: Option<setting::Shell<'src>>,
  pub(crate) windows_powershell: bool,
}

impl<'src> Settings<'src> {
  pub(crate) fn new() -> Settings<'src> {
    Settings {
      dotenv_load: None,
      export: false,
      positional_arguments: false,
      shell: None,
      windows_powershell: false,
    }
  }

  pub(crate) fn shell_command(&self, config: &Config) -> Command {
    let mut cmd = Command::new(self.shell_binary(config));

    cmd.args(self.shell_arguments(config));
Repository: casey/just
File: tests/lib.rs (Rust)

Code before:
mod examples;
mod export;
mod fmt;
mod functions;
mod init;
mod interrupts;
mod invocation_directory;
mod json;
mod line_prefixes;
mod misc;
mod positional_arguments;
mod quiet;
mod quote;
mod readme;
mod regexes;
mod search;
mod shebang;
mod shell;
mod show;
mod string;
mod sublime_syntax;
mod subsequents;
mod tempdir;
mod undefined_variables;
mod windows_powershell;
mod working_directory;


Review comment (suggestion):
```suggestion
#[cfg(target_family = "windows")]
mod windows_powershell;
```

Code after:
mod examples;
mod export;
mod fmt;
mod functions;
mod init;
mod interrupts;
mod invocation_directory;
mod json;
mod line_prefixes;
mod misc;
mod positional_arguments;
mod quiet;
mod quote;
mod readme;
mod regexes;
mod search;
mod shebang;
mod shell;
mod show;
mod string;
mod sublime_syntax;
mod subsequents;
mod tempdir;
mod undefined_variables;
#[cfg(target_family = "windows")]
mod windows_powershell;
mod working_directory;
Repository: casey/just
File: src/settings.rs (Rust)

Code before:
    if let (Some(shell), false) = (&self.shell, shell_or_args_present) {
      shell
        .arguments
        .iter()
        .map(|argument| argument.cooked.as_ref())
        .collect()
    } else if let Some(shell_args) = &config.shell_args {
      shell_args.iter().map(String::as_ref).collect()
    } else if cfg!(windows) && self.windows_powershell {
      WINDOWS_DEFAULT_SHELL_ARGS.to_vec()
    } else {
      DEFAULT_SHELL_ARG.to_vec()
    }
  }
}

#[cfg(test)]
mod tests {
  use crate::setting::Shell;

  use super::*;

  #[test]
  fn default_shell() {
    let mut settings = Settings::new();
    settings.windows_powershell = false;

    let config = Config {
      shell_command: false,
      ..testing::config(&[])
    };

    assert_eq!(settings.shell_binary(&config), "sh");
    assert_eq!(settings.shell_arguments(&config), vec!["-cu"]);
  }

  #[test]
  fn default_shell_powershell() {
    let mut settings = Settings::new();
    settings.windows_powershell = true;

    let config = Config {
      shell_command: false,
      ..testing::config(&[])
    };

    if cfg!(windows) {
      assert_eq!(settings.shell_binary(&config), "powershell.exe");
      assert_eq!(
        settings.shell_arguments(&config),
        vec!["-NoLogo", "-Command"]

Review comment (suggestion):
This defaults to false, so it can be omitted:

```suggestion
```

Code after:
    if let (Some(shell), false) = (&self.shell, shell_or_args_present) {
      shell
        .arguments
        .iter()
        .map(|argument| argument.cooked.as_ref())
        .collect()
    } else if let Some(shell_args) = &config.shell_args {
      shell_args.iter().map(String::as_ref).collect()
    } else if cfg!(windows) && self.windows_powershell {
      WINDOWS_POWERSHELL_ARGS.to_vec()
    } else {
      DEFAULT_SHELL_ARGS.to_vec()
    }
  }
}

#[cfg(test)]
mod tests {
  use crate::setting::Shell;

  use super::*;

  #[test]
  fn default_shell() {
    let settings = Settings::new();

    let config = Config {
      shell_command: false,
      ..testing::config(&[])
    };

    assert_eq!(settings.shell_binary(&config), "sh");
    assert_eq!(settings.shell_arguments(&config), vec!["-cu"]);
  }

  #[test]
  fn default_shell_powershell() {
    let mut settings = Settings::new();
    settings.windows_powershell = true;

    let config = Config {
      shell_command: false,
      ..testing::config(&[])
    };

    if cfg!(windows) {
      assert_eq!(settings.shell_binary(&config), "powershell.exe");
      assert_eq!(
        settings.shell_arguments(&config),
        vec!["-NoLogo", "-Command"]
      );
Repository: casey/just
File: src/settings.rs (Rust)

Code before:

  #[test]
  fn default_shell_powershell() {
    let mut settings = Settings::new();
    settings.windows_powershell = true;

    let config = Config {
      shell_command: false,
      ..testing::config(&[])
    };

    if cfg!(windows) {
      assert_eq!(settings.shell_binary(&config), "powershell.exe");
      assert_eq!(
        settings.shell_arguments(&config),
        vec!["-NoLogo", "-Command"]
      );
    } else {
      assert_eq!(settings.shell_binary(&config), "sh");
      assert_eq!(settings.shell_arguments(&config), vec!["-cu"]);
    }
  }

  #[test]
  fn overwrite_shell() {
    let mut settings = Settings::new();

    let config = Config {
      shell_command: true,
      shell: Some("lol".to_string()),
      shell_args: Some(vec!["-nice".to_string()]),
      ..testing::config(&[])
    };

    assert_eq!(settings.shell_binary(&config), "lol");
    assert_eq!(settings.shell_arguments(&config), vec!["-nice"]);
  }

  #[test]
  fn overwrite_shell_powershell() {
    let mut settings = Settings::new();
    settings.windows_powershell = true;

    let config = Config {
      shell_command: true,
      shell: Some("lol".to_string()),
      shell_args: Some(vec!["-nice".to_string()]),
      ..testing::config(&[])
    };

    assert_eq!(settings.shell_binary(&config), "lol");

Review comment (suggestion):
```suggestion
    let settings = Settings::new();
```

Code after:

  #[test]
  fn default_shell_powershell() {
    let mut settings = Settings::new();
    settings.windows_powershell = true;

    let config = Config {
      shell_command: false,
      ..testing::config(&[])
    };

    if cfg!(windows) {
      assert_eq!(settings.shell_binary(&config), "powershell.exe");
      assert_eq!(
        settings.shell_arguments(&config),
        vec!["-NoLogo", "-Command"]
      );
    } else {
      assert_eq!(settings.shell_binary(&config), "sh");
      assert_eq!(settings.shell_arguments(&config), vec!["-cu"]);
    }
  }

  #[test]
  fn overwrite_shell() {
    let settings = Settings::new();

    let config = Config {
      shell_command: true,
      shell: Some("lol".to_string()),
      shell_args: Some(vec!["-nice".to_string()]),
      ..testing::config(&[])
    };

    assert_eq!(settings.shell_binary(&config), "lol");
    assert_eq!(settings.shell_arguments(&config), vec!["-nice"]);
  }

  #[test]
  fn overwrite_shell_powershell() {
    let mut settings = Settings::new();
    settings.windows_powershell = true;

    let config = Config {
      shell_command: true,
      shell: Some("lol".to_string()),
      shell_args: Some(vec!["-nice".to_string()]),
      ..testing::config(&[])
    };

    assert_eq!(settings.shell_binary(&config), "lol");
Repository: casey/just
File: tests/json.rs (Rust)

Code before:
          "quiet": false,
          "shebang": false,
        }
      },
      "settings": {
        "allow_duplicate_recipes": false,
        "dotenv_load": null,
        "export": false,
        "positional_arguments": false,
        "shell": null,
        "windows_powershell": false,
      },
      "warnings": [],
    }),
  );
}

#[test]
fn duplicate_recipes() {
  test(
    "
      set allow-duplicate-recipes
      alias f := foo

      foo:
      foo:
    ",
    json!({
      "first": "foo",
      "aliases": {
        "f": {
          "name": "f",
          "target": "foo",
        }
      },
      "assignments": {},
      "recipes": {
        "foo": {
          "body": [],
          "dependencies": [],
          "doc": null,
          "name": "foo",
          "parameters": [],
          "priors": 0,
          "private": false,
          "quiet": false,
          "shebang": false,
        }
      },
      "settings": {
        "allow_duplicate_recipes": true,

Review comment (suggestion):
This test is slightly improved if the two `foo`s are different, so you can see the correct one in the output:

```suggestion
      foo:
      foo bar:
```

Code after:
          "quiet": false,
          "shebang": false,
        }
      },
      "settings": {
        "allow_duplicate_recipes": false,
        "dotenv_load": null,
        "export": false,
        "positional_arguments": false,
        "shell": null,
        "windows_powershell": false,
      },
      "warnings": [],
    }),
  );
}

#[test]
fn duplicate_recipes() {
  test(
    "
      set allow-duplicate-recipes
      alias f := foo

      foo:
      foo bar:
    ",
    json!({
      "first": "foo",
      "aliases": {
        "f": {
          "name": "f",
          "target": "foo",
        }
      },
      "assignments": {},
      "recipes": {
        "foo": {
          "body": [],
          "dependencies": [],
          "doc": null,
          "name": "foo",
          "parameters": [
            {
              "name": "bar",
              "export": false,
              "default": null,
              "kind": "singular",
            },
          ],
          "priors": 0,
Repository: casey/just
File: tests/misc.rs (Rust)

Code before:
  |       ^^^
",
  status:   EXIT_FAILURE,
}

test! {
  name:     duplicate_recipe,
  justfile: "b:\nb:",
  args:     ("b"),
  stdout:   "",
  stderr:   "error: Recipe `b` first defined on line 1 is redefined on line 2
  |
2 | b:
  | ^
",
  status:   EXIT_FAILURE,
}

test! {
  name:     allow_duplicate_recipes,
  justfile: "b:\n  echo foo\nb:\n  echo bar\nset allow-duplicate-recipes",
  args:     ("b"),
  stdout: "bar\n",
  stderr: "echo bar\n",
}

test! {
  name:     duplicate_variable,
  justfile: "a := 'hello'\na := 'hello'\nfoo:",
  args:     ("foo"),
  stdout:   "",
  stderr:   "error: Variable `a` has multiple definitions
  |
2 | a := 'hello'
  | ^
",
  status:   EXIT_FAILURE,
}

test! {
  name:     unexpected_token_in_dependency_position,
  justfile: "foo: 'bar'",
  args:     ("foo"),
  stdout:   "",
  stderr:   "error: Expected '&&', comment, end of file, end of line, \
    identifier, or '(', but found string
  |
1 | foo: 'bar'
  |      ^^^^^
",
  status:   EXIT_FAILURE,

Review comment (refactor):
I try not to add new tests to `misc.rs`, since it's way too long as is. You can create a new file, `tests/allow_duplicate_recipes.rs` and put the test in there. Also, I try not to add new macro-based tests, i.e., using the `test!` macro.  Check out one of the struct-based tests that uses `Test::new`.

Code after:
  |       ^^^
",
  status:   EXIT_FAILURE,
}

test! {
  name:     duplicate_recipe,
  justfile: "b:\nb:",
  args:     ("b"),
  stdout:   "",
  stderr:   "error: Recipe `b` first defined on line 1 is redefined on line 2
  |
2 | b:
  | ^
",
  status:   EXIT_FAILURE,
}

test! {
  name:     duplicate_variable,
  justfile: "a := 'hello'\na := 'hello'\nfoo:",
  args:     ("foo"),
  stdout:   "",
  stderr:   "error: Variable `a` has multiple definitions
  |
2 | a := 'hello'
  | ^
",
  status:   EXIT_FAILURE,
}

test! {
  name:     unexpected_token_in_dependency_position,
  justfile: "foo: 'bar'",
  args:     ("foo"),
  stdout:   "",
  stderr:   "error: Expected '&&', comment, end of file, end of line, \
    identifier, or '(', but found string
  |
1 | foo: 'bar'
  |      ^^^^^
",
  status:   EXIT_FAILURE,
}

test! {
  name:     unexpected_token_after_name,
  justfile: "foo 'bar'",
  args:     ("foo"),
  stdout:   "",
  stderr:   "error: Expected '*', ':', '$', identifier, or '+', but found string
Repository: casey/just
File: tests/functions.rs (Rust)

Code before:
}

#[test]
fn join_argument_count_error() {
  Test::new()
    .justfile("x := join('a')")
    .args(&["--evaluate"])
    .stderr(
      "
      error: Function `join` called with 1 argument but takes 2 or more
        |
      1 | x := join(\'a\')
        |      ^^^^
      ",
    )
    .status(EXIT_FAILURE)
    .run();
}

#[test]
fn test_path_exists_filepath_exist() {
  let test = Test::new();
  let testfilepathbuf = test.tempdir.path().join("testfile.txt");
  let testfilepath = testfilepathbuf.as_path();
  let _file = std::fs::File::create(testfilepath);
  test
    .justfile(format!(
      "x := path_exists(\"{}\")",
      testfilepath.to_str().unwrap()
    ))
    .args(&["--evaluate", "x"])
    .stdout("true")
    .unindent_stdout(false)
    .run();
}

#[test]
fn test_path_exists_filepath_doesnt_exist() {
  let test = Test::new();
  let testfilepathbuf = test.tempdir.path().join("testfile.txt");
  let testfilepath = testfilepathbuf.as_path();
  test
    .justfile(format!(
      "x := path_exists(\"{}\")",
      testfilepath.to_str().unwrap()
    ))
    .args(&["--evaluate", "x"])
    .stdout("false")
    .unindent_stdout(false)
    .run();
}

Review comment (suggestion):
You can use `Test::tree` to populate the tempdir the test runs in with files:

```suggestion
  Test::new()
    .tree(tree! {
      testfile: ""
    })
```

Code after:
}

#[test]
fn join_argument_count_error() {
  Test::new()
    .justfile("x := join('a')")
    .args(&["--evaluate"])
    .stderr(
      "
      error: Function `join` called with 1 argument but takes 2 or more
        |
      1 | x := join(\'a\')
        |      ^^^^
      ",
    )
    .status(EXIT_FAILURE)
    .run();
}

#[test]
fn test_path_exists_filepath_exist() {
  Test::new()
    .tree(tree! {
      testfile: ""
    })
    .justfile("x := path_exists('testfile')")
    .args(&["--evaluate", "x"])
    .stdout("true")
    .run();
}

#[test]
fn test_path_exists_filepath_doesnt_exist() {
  Test::new()
    .justfile("x := path_exists('testfile')")
    .args(&["--evaluate", "x"])
    .stdout("false")
    .run();
}
Repository: casey/just
File: tests/functions.rs (Rust)

Code before:
  Test::new()
    .justfile("x := join('a')")
    .args(&["--evaluate"])
    .stderr(
      "
      error: Function `join` called with 1 argument but takes 2 or more
        |
      1 | x := join(\'a\')
        |      ^^^^
      ",
    )
    .status(EXIT_FAILURE)
    .run();
}

#[test]
fn test_path_exists_filepath_exist() {
  let test = Test::new();
  let testfilepathbuf = test.tempdir.path().join("testfile.txt");
  let testfilepath = testfilepathbuf.as_path();
  let _file = std::fs::File::create(testfilepath);
  test
    .justfile(format!(
      "x := path_exists(\"{}\")",
      testfilepath.to_str().unwrap()
    ))
    .args(&["--evaluate", "x"])
    .stdout("true")
    .unindent_stdout(false)
    .run();
}

#[test]
fn test_path_exists_filepath_doesnt_exist() {
  let test = Test::new();
  let testfilepathbuf = test.tempdir.path().join("testfile.txt");
  let testfilepath = testfilepathbuf.as_path();
  test
    .justfile(format!(
      "x := path_exists(\"{}\")",
      testfilepath.to_str().unwrap()
    ))
    .args(&["--evaluate", "x"])
    .stdout("false")
    .unindent_stdout(false)
    .run();
}

#[test]
fn test_path_exists_dirpath_exist() {
  let test = Test::new();

Review comment (suggestion):
You should just be able to do:

```suggestion
    .justfile("x := path_exists('testfile')")
```

Code after:
  Test::new()
    .justfile("x := join('a')")
    .args(&["--evaluate"])
    .stderr(
      "
      error: Function `join` called with 1 argument but takes 2 or more
        |
      1 | x := join(\'a\')
        |      ^^^^
      ",
    )
    .status(EXIT_FAILURE)
    .run();
}

#[test]
fn test_path_exists_filepath_exist() {
  Test::new()
    .tree(tree! {
      testfile: ""
    })
    .justfile("x := path_exists('testfile')")
    .args(&["--evaluate", "x"])
    .stdout("true")
    .run();
}

#[test]
fn test_path_exists_filepath_doesnt_exist() {
  Test::new()
    .justfile("x := path_exists('testfile')")
    .args(&["--evaluate", "x"])
    .stdout("false")
    .run();
}