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

Code before:
  }

  fn context(&self) -> Option<Token<'src>> {
    match self {
      Self::Backtick { token, .. } => Some(*token),
      Self::Compile { compile_error } => Some(compile_error.context()),
      Self::FunctionCall { function, .. } => Some(function.token()),
      _ => None,
    }
  }

  pub(crate) fn internal(message: impl Into<String>) -> Self {
    Self::Internal {
      message: message.into(),
    }
  }

  pub(crate) fn suppress_message(&self) -> bool {
    match self {
      Error::Code {
        suppress_message, ..
      } => *suppress_message,
      _ => false,
    }
  }
}

impl<'src> From<CompileError<'src>> for Error<'src> {
  fn from(compile_error: CompileError<'src>) -> Self {
    Self::Compile { compile_error }
  }
}

impl<'src> From<ConfigError> for Error<'src> {
  fn from(config_error: ConfigError) -> Self {
    Self::Config { config_error }
  }
}

impl<'src> From<dotenv::Error> for Error<'src> {
  fn from(dotenv_error: dotenv::Error) -> Error<'src> {
    Self::Dotenv { dotenv_error }
  }
}

impl<'src> From<SearchError> for Error<'src> {
  fn from(search_error: SearchError) -> Self {
    Self::Search { search_error }
  }
}


Review comment (refactor):
I'm not 100% sure this is the right syntax, but it should be possible to simplify this like so:

```suggestion
    matches!(self, Error::Code { suppress_message: true, ..})
```

Code after:
  }

  fn context(&self) -> Option<Token<'src>> {
    match self {
      Self::Backtick { token, .. } => Some(*token),
      Self::Compile { compile_error } => Some(compile_error.context()),
      Self::FunctionCall { function, .. } => Some(function.token()),
      _ => None,
    }
  }

  pub(crate) fn internal(message: impl Into<String>) -> Self {
    Self::Internal {
      message: message.into(),
    }
  }

  pub(crate) fn suppress_message(&self) -> bool {
    matches!(
      self,
      Error::Code {
        suppress_message: true,
        ..
      }
    )
  }
}

impl<'src> From<CompileError<'src>> for Error<'src> {
  fn from(compile_error: CompileError<'src>) -> Self {
    Self::Compile { compile_error }
  }
}

impl<'src> From<ConfigError> for Error<'src> {
  fn from(config_error: ConfigError) -> Self {
    Self::Config { config_error }
  }
}

impl<'src> From<dotenv::Error> for Error<'src> {
  fn from(dotenv_error: dotenv::Error) -> Error<'src> {
    Self::Dotenv { dotenv_error }
  }
}

impl<'src> From<SearchError> for Error<'src> {
  fn from(search_error: SearchError) -> Self {
    Self::Search { search_error }
  }
}
Repository: casey/just
File: src/justfile.rs (Rust)

Code before:
      }
    ));
    }
  }

  // This test exists to make sure that shebang recipes run correctly.  Although
  // this script is still executed by a shell its behavior depends on the value of
  // a variable and continuing even though a command fails, whereas in plain
  // recipes variables are not available in subsequent lines and execution stops
  // when a line fails.
  run_error! {
    name: run_shebang,
    src: "
      a:
        #!/usr/bin/env sh
        code=200
          x() { return $code; }
            x
              x
    ",
    args: ["a"],
    error: Code {
      recipe,
      line_number,
      code,
      ..
    },
    check: {
      assert_eq!(recipe, "a");
      assert_eq!(code, 200);
      assert_eq!(line_number, None);
    }
  }

  run_error! {
    name: code_error,
    src: "
      fail:
        @exit 100
    ",
    args: ["fail"],
    error: Code {
      recipe,
      line_number,
      code,
      ..
    },
    check: {
      assert_eq!(recipe, "fail");
      assert_eq!(code, 100);
      assert_eq!(line_number, Some(2));

Review comment (refactor):
It's more verbose, but I think I'd extract and check these with an assert below:

```suggestion
      suppress_error_message,
```

Code after:
      }
    ));
    }
  }

  // This test exists to make sure that shebang recipes run correctly.  Although
  // this script is still executed by a shell its behavior depends on the value of
  // a variable and continuing even though a command fails, whereas in plain
  // recipes variables are not available in subsequent lines and execution stops
  // when a line fails.
  run_error! {
    name: run_shebang,
    src: "
      a:
        #!/usr/bin/env sh
        code=200
          x() { return $code; }
            x
              x
    ",
    args: ["a"],
    error: Code {
      recipe,
      line_number,
      code,
      suppress_message,
    },
    check: {
      assert_eq!(recipe, "a");
      assert_eq!(code, 200);
      assert_eq!(line_number, None);
      assert!(!suppress_message);
    }
  }

  run_error! {
    name: code_error,
    src: "
      fail:
        @exit 100
    ",
    args: ["fail"],
    error: Code {
      recipe,
      line_number,
      code,
      suppress_message,
    },
    check: {
      assert_eq!(recipe, "fail");
      assert_eq!(code, 100);
Repository: casey/just
File: src/parser.rs (Rust)

Code before:

    if self.accepted(Comma)? {
      while !self.next_is(BracketR) {
        arguments.push(self.parse_string_literal()?);

        if !self.accepted(Comma)? {
          break;
        }
      }
    }

    self.expect(BracketR)?;

    Ok(Shell { arguments, command })
  }

  /// Parse a recipe attribute
  fn parse_attribute(&mut self) -> CompileResult<'src, Attribute> {
    let token = self.expect(Identifier)?;
    let lexeme = token.lexeme();
    if let Some(attribute) = Attribute::from_lexeme(lexeme) {
      self.expect(BracketR)?;
      Ok(attribute)
    } else {
      Err(token.error(CompileErrorKind::UnknownAttribute { attribute: lexeme }))
    }
  }
}

#[cfg(test)]
mod tests {
  use super::*;

  use pretty_assertions::assert_eq;
  use CompileErrorKind::*;

  macro_rules! test {
    {
      name: $name:ident,
      text: $text:expr,
      tree: $tree:tt,
    } => {
      #[test]
      fn $name() {
        let text: String = $text.into();
        let want = tree!($tree);
        test(&text, want);
      }
    }
  }


Review comment (suggestion):
This is not quite right, since you also need the token to produce the error message, but something like this:

```suggestion
  /// Parse a recipe attribute name
  fn parse_attribute_name(&mut self) -> CompileResult<'src, Attribute> {
    let name = self.parse_name()?;
    Attribute::from_lexeme(name).ok_or_else(|| token.error(CompileErrorKind::UnknownAttribute { attribute: lexeme }))
    }
```

Code after:
    let mut arguments = Vec::new();

    if self.accepted(Comma)? {
      while !self.next_is(BracketR) {
        arguments.push(self.parse_string_literal()?);

        if !self.accepted(Comma)? {
          break;
        }
      }
    }

    self.expect(BracketR)?;

    Ok(Shell { arguments, command })
  }

  /// Parse a recipe attribute name
  fn parse_attribute_name(&mut self) -> CompileResult<'src, Attribute> {
    let name = self.parse_name()?;
    Attribute::from_name(name).ok_or_else(|| {
      name.error(CompileErrorKind::UnknownAttribute {
        attribute: name.lexeme(),
      })
    })
  }
}

#[cfg(test)]
mod tests {
  use super::*;

  use pretty_assertions::assert_eq;
  use CompileErrorKind::*;

  macro_rules! test {
    {
      name: $name:ident,
      text: $text:expr,
      tree: $tree:tt,
    } => {
      #[test]
      fn $name() {
        let text: String = $text.into();
        let want = tree!($tree);
        test(&text, want);
      }
    }
  }

  fn test(text: &str, want: Tree) {
Repository: casey/just
File: src/setting.rs (Rust)

Code before:
#[derive(Debug, Clone)]
pub(crate) enum Setting<'src> {
  AllowDuplicateRecipes(bool),
  DotenvLoad(bool),
  IgnoreComments(bool),
  Export(bool),
  PositionalArguments(bool),
  Shell(Shell<'src>),
  Tempdir(StringLiteral<'src>),
  WindowsPowerShell(bool),
  WindowsShell(Shell<'src>),
}

impl<'src> Display for Setting<'src> {
  fn fmt(&self, f: &mut Formatter) -> Result<(), fmt::Error> {
    match self {
      Setting::AllowDuplicateRecipes(value)
      | Setting::DotenvLoad(value)
      | Setting::IgnoreComments(value)
      | Setting::Export(value)
      | Setting::PositionalArguments(value)
      | Setting::WindowsPowerShell(value) => write!(f, "{}", value),
      Setting::Shell(shell) | Setting::WindowsShell(shell) => write!(f, "{}", shell),
      Setting::Tempdir(path) => write!(f, "{}", path),
    }
  }
}


Review comment (suggestion):
```suggestion
      Setting::Tempdir(tempdir) => write!(f, "{}", tempdir),
```

Code after:
#[derive(Debug, Clone)]
pub(crate) enum Setting<'src> {
  AllowDuplicateRecipes(bool),
  DotenvLoad(bool),
  Export(bool),
  Fallback(bool),
  IgnoreComments(bool),
  PositionalArguments(bool),
  Shell(Shell<'src>),
  Tempdir(String),
  WindowsPowerShell(bool),
  WindowsShell(Shell<'src>),
}

impl<'src> Display for Setting<'src> {
  fn fmt(&self, f: &mut Formatter) -> Result<(), fmt::Error> {
    match self {
      Setting::AllowDuplicateRecipes(value)
      | Setting::DotenvLoad(value)
      | Setting::Export(value)
      | Setting::Fallback(value)
      | Setting::IgnoreComments(value)
      | Setting::PositionalArguments(value)
      | Setting::WindowsPowerShell(value) => write!(f, "{}", value),
      Setting::Shell(shell) | Setting::WindowsShell(shell) => write!(f, "{}", shell),
      Setting::Tempdir(tempdir) => {
        write!(f, "{:?}", tempdir)
      }
    }
  }
}
Repository: casey/just
File: src/settings.rs (Rust)

Code before:
use super::*;

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, Default)]
pub(crate) struct Settings<'src> {
  pub(crate) allow_duplicate_recipes: bool,
  pub(crate) dotenv_load: Option<bool>,
  pub(crate) export: bool,
  pub(crate) ignore_comments: bool,
  pub(crate) positional_arguments: bool,
  pub(crate) shell: Option<Shell<'src>>,
  pub(crate) temp_dir: Option<StringLiteral<'src>>,
  pub(crate) windows_powershell: bool,
  pub(crate) windows_shell: Option<Shell<'src>>,
}

impl<'src> Settings<'src> {
  pub(crate) fn shell_command(&self, config: &Config) -> Command {
    let (command, args) = self.shell(config);

    let mut cmd = Command::new(command);

    cmd.args(args);

    cmd
  }

  pub(crate) fn shell<'a>(&'a self, config: &'a Config) -> (&'a str, Vec<&'a str>) {
    match (&config.shell, &config.shell_args) {
      (Some(shell), Some(shell_args)) => (shell, shell_args.iter().map(String::as_ref).collect()),
      (Some(shell), None) => (shell, DEFAULT_SHELL_ARGS.to_vec()),
      (None, Some(shell_args)) => (
        DEFAULT_SHELL,
        shell_args.iter().map(String::as_ref).collect(),
      ),
      (None, None) => {
        if let (true, Some(shell)) = (cfg!(windows), &self.windows_shell) {
          (

Review comment (suggestion):
```suggestion
  pub(crate) tempdir: Option<String>,
```

Code after:
use super::*;

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, Default)]
pub(crate) struct Settings<'src> {
  pub(crate) allow_duplicate_recipes: bool,
  pub(crate) dotenv_load: Option<bool>,
  pub(crate) export: bool,
  pub(crate) fallback: bool,
  pub(crate) ignore_comments: bool,
  pub(crate) positional_arguments: bool,
  pub(crate) shell: Option<Shell<'src>>,
  pub(crate) tempdir: Option<String>,
  pub(crate) windows_powershell: bool,
  pub(crate) windows_shell: Option<Shell<'src>>,
}

impl<'src> Settings<'src> {
  pub(crate) fn shell_command(&self, config: &Config) -> Command {
    let (command, args) = self.shell(config);

    let mut cmd = Command::new(command);

    cmd.args(args);

    cmd
  }

  pub(crate) fn shell<'a>(&'a self, config: &'a Config) -> (&'a str, Vec<&'a str>) {
    match (&config.shell, &config.shell_args) {
      (Some(shell), Some(shell_args)) => (shell, shell_args.iter().map(String::as_ref).collect()),
      (Some(shell), None) => (shell, DEFAULT_SHELL_ARGS.to_vec()),
      (None, Some(shell_args)) => (
        DEFAULT_SHELL,
        shell_args.iter().map(String::as_ref).collect(),
      ),
      (None, None) => {
        if let (true, Some(shell)) = (cfg!(windows), &self.windows_shell) {
Repository: casey/just
File: tests/misc.rs (Rust)

Code before:
    name: custom_tempdir,
    justfile: r#"
    set tempdir := "."
    parent_tempdir := "."
    foo:
        #!/usr/bin/env bash
        script_tempdir=$(basename "$(dirname $0)")
        script_foo=`realpath {{parent_tempdir}}`/$script_tempdir/foo
        [[ -f $script_foo ]] && echo "foo_exists" || echo "foo_missing"
    "#,
    stdout: "foo_exists\n",
}

test! {
    name: standard_tempdir,
    justfile: r#"
    standard_parent_tempdir := "/tmp"
    foo:
        #!/usr/bin/env bash
        script_tempdir=$(basename "$(dirname $0)")
        script_foo={{standard_parent_tempdir}}/$script_tempdir/foo
        [[ -f $script_foo ]] && echo "foo_exists" || echo "foo_missing"
    "#,
    stdout: "foo_exists\n",
}

test! {
  name:     shebang_line_numbers,
  justfile: r#"
    quiet:
      #!/usr/bin/env cat

      a

      b


      c


  "#,
  stdout:   "
    #!/usr/bin/env cat


    a

    b


    c

Review comment (suggestion):
A few comments:

- There are still a bunch of macro-based tests, but I've been moving away from them. For new tests it's best to use the `Test` struct. Check out some of the other tests.
- These tests can go in their own file, perhaps `tests/tempdir.rs`.
- `realpath` isn't available on MacOS.
- The test can be simplified with something like this:

This will only succeed if the temporary directory is the same directory as the current directory, which will only be true if the tempdir has been set:

```
set tempdir := "."
foo:
  touch $(basename "$(dirname $0)")/bar
  cat bar
```

- I think the `standard_tempdir` test can be removed.

Code after:
  name:     shebang_line_numbers,
  justfile: r#"
    quiet:
      #!/usr/bin/env cat

      a

      b


      c


  "#,
  stdout:   "
    #!/usr/bin/env cat


    a

    b


    c
  ",
}

test! {
  name:     complex_dependencies,
  justfile: r#"
a: b
b:
c: b a
"#,
  args:     ("b"),
  stdout:   "",
}

test! {
  name:     parameter_shadows_variable,
  justfile: "FOO := 'hello'\na FOO:",
  args:     ("a"),
  stdout:   "",
  stderr:   "error: Parameter `FOO` shadows variable of the same name
  |
2 | a FOO:
  |   ^^^
",
  status:   EXIT_FAILURE,
}
Repository: casey/just
File: src/function.rs (Rust)

Code before:
    }
  }
}

fn absolute_path(context: &FunctionContext, path: &str) -> Result<String, String> {
  let abs_path_unchecked = context.search.working_directory.join(path).lexiclean();
  match abs_path_unchecked.to_str() {
    Some(absolute_path) => Ok(absolute_path.to_owned()),
    None => Err(format!(
      "Working directory is not valid unicode: {}",
      context.search.working_directory.display()
    )),
  }
}

fn arch(_context: &FunctionContext) -> Result<String, String> {
  Ok(target::arch().to_owned())
}

fn capitalize(_context: &FunctionContext, s: &str) -> Result<String, String> {
  let mut chars = s.chars();
  match chars.next() {
      None => Ok(String::new()),
      Some(c) => Ok(c.to_uppercase().collect::<String>() +
                    &chars.as_str().to_lowercase()),
  }
}

fn clean(_context: &FunctionContext, path: &str) -> Result<String, String> {
  Ok(Path::new(path).lexiclean().to_str().unwrap().to_owned())
}

fn env_var(context: &FunctionContext, key: &str) -> Result<String, String> {
  use std::env::VarError::*;

  if let Some(value) = context.dotenv.get(key) {
    return Ok(value.clone());
  }

  match env::var(key) {
    Err(NotPresent) => Err(format!("environment variable `{}` not present", key)),
    Err(NotUnicode(os_string)) => Err(format!(
      "environment variable `{}` not unicode: {:?}",
      key, os_string
    )),
    Ok(value) => Ok(value),
  }
}

fn env_var_or_default(
  context: &FunctionContext,

Review comment (suggestion):
Maybe something like this:

```suggestion
  Ok(s.chars().enumerate().flat_map(|(i, c)| if i == 0 { c.upper_case() } else { c.lower_case() }).collect())
```

Code after:
    }
  }
}

fn absolute_path(context: &FunctionContext, path: &str) -> Result<String, String> {
  let abs_path_unchecked = context.search.working_directory.join(path).lexiclean();
  match abs_path_unchecked.to_str() {
    Some(absolute_path) => Ok(absolute_path.to_owned()),
    None => Err(format!(
      "Working directory is not valid unicode: {}",
      context.search.working_directory.display()
    )),
  }
}

fn arch(_context: &FunctionContext) -> Result<String, String> {
  Ok(target::arch().to_owned())
}

fn capitalize(_context: &FunctionContext, s: &str) -> Result<String, String> {
  Ok(
    s.chars()
      .enumerate()
      .flat_map(|(i, c)| {
        if i == 0 {
          c.to_uppercase().collect::<Vec<_>>()
        } else {
          c.to_lowercase().collect::<Vec<_>>()
        }
      })
      .collect(),
  )
}

fn clean(_context: &FunctionContext, path: &str) -> Result<String, String> {
  Ok(Path::new(path).lexiclean().to_str().unwrap().to_owned())
}

fn env_var(context: &FunctionContext, key: &str) -> Result<String, String> {
  use std::env::VarError::*;

  if let Some(value) = context.dotenv.get(key) {
    return Ok(value.clone());
  }

  match env::var(key) {
    Err(NotPresent) => Err(format!("environment variable `{}` not present", key)),
    Err(NotUnicode(os_string)) => Err(format!(
      "environment variable `{}` not unicode: {:?}",
      key, os_string
    )),
Repository: casey/just
File: src/error.rs (Rust)

Code before:
  },
  EditorStatus {
    editor: OsString,
    status: ExitStatus,
  },
  EvalUnknownVariable {
    variable: String,
    suggestion: Option<Suggestion<'src>>,
  },
  FormatCheckFoundDiff,
  FunctionCall {
    function: Name<'src>,
    message: String,
  },
  InitExists {
    justfile: PathBuf,
  },
  IncludeRecursive {
    cur_path: PathBuf,
    recursively_included_path: PathBuf,
  },
  Internal {
    message: String,
  },
  InvalidInclude {
    line_number: usize,
  },
  Io {
    recipe: &'src str,
    io_error: io::Error,
  },
  Load {
    path: PathBuf,
    io_error: io::Error,
  },
  NoChoosableRecipes,
  NoRecipes,
  RegexCompile {
    source: regex::Error,
  },
  Search {
    search_error: SearchError,
  },
  Shebang {
    recipe: &'src str,
    command: String,
    argument: Option<String>,
    io_error: io::Error,
  },
  Signal {
    recipe: &'src str,

Review comment (suggestion):
We should include the path and the line number, since this error might originate from an include, and not the root justfile.

Code after:
  },
  EditorInvoke {
    editor: OsString,
    io_error: io::Error,
  },
  EditorStatus {
    editor: OsString,
    status: ExitStatus,
  },
  EvalUnknownVariable {
    variable: String,
    suggestion: Option<Suggestion<'src>>,
  },
  FormatCheckFoundDiff,
  FunctionCall {
    function: Name<'src>,
    message: String,
  },
  IncludeMissingPath {
    file: PathBuf,
    line: usize,
  },
  InitExists {
    justfile: PathBuf,
  },
  Internal {
    message: String,
  },
  InvalidDirective {
    line: String,
  },
  Io {
    recipe: &'src str,
    io_error: io::Error,
  },
  Load {
    path: PathBuf,
    io_error: io::Error,
  },
  NoChoosableRecipes,
  NoRecipes,
  RegexCompile {
    source: regex::Error,
  },
  Search {
    search_error: SearchError,
  },
  Shebang {
    recipe: &'src str,
    command: String,
    argument: Option<String>,
Repository: casey/just
File: src/error.rs (Rust)

Code before:
    io_error: io::Error,
  },
  EditorStatus {
    editor: OsString,
    status: ExitStatus,
  },
  EvalUnknownVariable {
    variable: String,
    suggestion: Option<Suggestion<'src>>,
  },
  FormatCheckFoundDiff,
  FunctionCall {
    function: Name<'src>,
    message: String,
  },
  InitExists {
    justfile: PathBuf,
  },
  IncludeRecursive {
    cur_path: PathBuf,
    recursively_included_path: PathBuf,
  },
  Internal {
    message: String,
  },
  InvalidInclude {
    line_number: usize,
  },
  Io {
    recipe: &'src str,
    io_error: io::Error,
  },
  Load {
    path: PathBuf,
    io_error: io::Error,
  },
  NoChoosableRecipes,
  NoRecipes,
  RegexCompile {
    source: regex::Error,
  },
  Search {
    search_error: SearchError,
  },
  Shebang {
    recipe: &'src str,
    command: String,
    argument: Option<String>,
    io_error: io::Error,
  },
  Signal {

Review comment (suggestion):
I think we can improve this error. If it only prints the current path, and the path that we've already seen, it might not be clear why we've already seen this path. Check out `CompileError::{CircularRecipeDependency, CircularVariableDependency}` for example formatting.

The ideal form would be something like:

```suggestion
  CircularInclude {
    prefix: Vec<PathBuf>,
    cycle: Vec<PathBuf>,
  },
```

`prefix` is the includes that got us to this point, and `cycle` is the circular inclusion that triggered the error.

The prefix helps the user find where the cycle started, and the cycle helps the user figure out what the actual cycle was.

Code after:
    serde_json_error: serde_json::Error,
  },
  EditorInvoke {
    editor: OsString,
    io_error: io::Error,
  },
  EditorStatus {
    editor: OsString,
    status: ExitStatus,
  },
  EvalUnknownVariable {
    variable: String,
    suggestion: Option<Suggestion<'src>>,
  },
  FormatCheckFoundDiff,
  FunctionCall {
    function: Name<'src>,
    message: String,
  },
  IncludeMissingPath {
    file: PathBuf,
    line: usize,
  },
  InitExists {
    justfile: PathBuf,
  },
  Internal {
    message: String,
  },
  InvalidDirective {
    line: String,
  },
  Io {
    recipe: &'src str,
    io_error: io::Error,
  },
  Load {
    path: PathBuf,
    io_error: io::Error,
  },
  NoChoosableRecipes,
  NoRecipes,
  RegexCompile {
    source: regex::Error,
  },
  Search {
    search_error: SearchError,
  },
  Shebang {
    recipe: &'src str,
    command: String,