ronantakizawa/codereview-bench huggingface dataset:
Repository: starship/starship
File: src/modules/sudo.rs (Rust)

Code before:

#[cfg(test)]
mod tests {
    use crate::test::ModuleRenderer;
    use ansi_term::Color;

    #[test]
    #[cfg(not(windows))]
    fn test_sudo_not_cached() {
        let actual = ModuleRenderer::new("sudo")
            .cmd("sudo -n true", None)
            .config(toml::toml! {
                [sudo]
                disabled = false
            })
            .collect();
        let expected = None;

        assert_eq!(expected, actual);
    }

    #[test]
    #[cfg(not(windows))]
    fn test_sudo_cached() {
        let actual = ModuleRenderer::new("sudo")
            .cmd("sudo -n true", None)
            .config(toml::toml! {
                [sudo]
                disabled = false
            })
            .collect();
        let expected = Some(format!("{}", Color::Blue.bold().paint("as 🧙 ")));

        assert_eq!(expected, actual);
    }

    #[test]
    #[cfg(windows)]
    fn test_allow_windows_disabled_blocks_windows() {
        let actual = ModuleRenderer::new("sudo")
            .cmd("sudo -n true", None)
            .config(toml::toml! {
                [sudo]
                disabled = false
                allow_windows = false
            })
            .collect();
        let expected = None;

        assert_eq!(expected, actual);
    }

Review comment (suggestion):
To fix the tests, you need to mock the expected value in every test and not only `None`. In this test, that would be the following:

```rust
Some(CommandOutput {
    stdout: "".to_owned(),
    stderr: "".to_owned(),
})
```



Code after:

#[cfg(test)]
mod tests {
    use crate::{test::ModuleRenderer, utils::CommandOutput};
    use ansi_term::Color;

    #[test]
    fn test_sudo_not_cached() {
        let actual = ModuleRenderer::new("sudo")
            .cmd("sudo -n true", None)
            .config(toml::toml! {
                [sudo]
                disabled = false
                allow_windows = true
            })
            .collect();
        let expected = None;

        assert_eq!(expected, actual);
    }

    #[test]
    fn test_sudo_cached() {
        let actual = ModuleRenderer::new("sudo")
            .cmd(
                "sudo -n true",
                Some(CommandOutput {
                    stdout: "".to_owned(),
                    stderr: "".to_owned(),
                }),
            )
            .config(toml::toml! {
                [sudo]
                disabled = false
                allow_windows = true
            })
            .collect();
        let expected = Some(format!("{}", Color::Blue.bold().paint("as 🧙 ")));

        assert_eq!(expected, actual);
    }

    #[test]
    #[cfg(windows)]
    fn test_allow_windows_disabled_blocks_windows() {
        let actual = ModuleRenderer::new("sudo")
            .cmd(
                "sudo -n true",
                Some(CommandOutput {
                    stdout: "".to_owned(),
                    stderr: "".to_owned(),
Repository: starship/starship
File: src/modules/sudo.rs (Rust)

Code before:
    #[test]
    #[cfg(not(windows))]
    fn test_sudo_cached() {
        let actual = ModuleRenderer::new("sudo")
            .cmd(
                "sudo -n true",
                Some(CommandOutput {
                    stdout: "".to_owned(),
                    stderr: "".to_owned(),
                }),
            )
            .config(toml::toml! {
                [sudo]
                disabled = false
            })
            .collect();
        let expected = Some(format!("{}", Color::Blue.bold().paint("as 🧙 ")));

        assert_eq!(expected, actual);
    }

    #[test]
    #[cfg(windows)]
    fn test_allow_windows_disabled_blocks_windows() {
        let actual = ModuleRenderer::new("sudo")
            .cmd("sudo -n true", None)
            .config(toml::toml! {
                [sudo]
                disabled = false
                allow_windows = false
            })
            .collect();
        let expected = None;

        assert_eq!(expected, actual);
    }
}


Review comment (suggestion):
For this test to work as intended, `sudo` needs to succeed to check if it's being blocked properly.

```suggestion
            .cmd(
                "sudo -n true",
                Some(CommandOutput {
                    stdout: "".to_owned(),
                    stderr: "".to_owned(),
                }),
             )
```

Code after:
    #[test]
    fn test_sudo_cached() {
        let actual = ModuleRenderer::new("sudo")
            .cmd(
                "sudo -n true",
                Some(CommandOutput {
                    stdout: "".to_owned(),
                    stderr: "".to_owned(),
                }),
            )
            .config(toml::toml! {
                [sudo]
                disabled = false
                allow_windows = true
            })
            .collect();
        let expected = Some(format!("{}", Color::Blue.bold().paint("as 🧙 ")));

        assert_eq!(expected, actual);
    }

    #[test]
    #[cfg(windows)]
    fn test_allow_windows_disabled_blocks_windows() {
        let actual = ModuleRenderer::new("sudo")
            .cmd(
                "sudo -n true",
                Some(CommandOutput {
                    stdout: "".to_owned(),
                    stderr: "".to_owned(),
                }),
            )
            .config(toml::toml! {
                [sudo]
                disabled = false
                allow_windows = false
            })
            .collect();
        let expected = None;

        assert_eq!(expected, actual);
    }
}
Repository: starship/starship
File: src/modules/sudo.rs (Rust)

Code before:

    if config.disabled {
        return None;
    }

    if !config.allow_windows && env::consts::FAMILY == "windows" {
        return None;
    }

    let is_sudo_cached = context.exec_cmd("sudo", &["-n", "true"]).is_some();

    if !is_sudo_cached {
        return None;
    }

    let parsed = StringFormatter::new(config.format).and_then(|formatter| {
        formatter
            .map_meta(|variable, _| match variable {
                "symbol" => Some(config.symbol),
                _ => None,
            })
            .map_style(|variable| match variable {
                "style" => Some(Ok(config.style)),
                _ => None,
            })
            .parse(None)
    });

    module.set_segments(match parsed {
        Ok(segments) => segments,
        Err(error) => {
            log::warn!("Error in module `sudo`:\n{}", error);
            return None;
        }
    });

    Some(module)
}

#[cfg(test)]
mod tests {
    use crate::{test::ModuleRenderer, utils::CommandOutput};
    use ansi_term::Color;

    #[test]
    #[cfg(not(windows))]
    fn test_sudo_not_cached() {
        let actual = ModuleRenderer::new("sudo")
            .cmd("sudo -n true", None)
            .config(toml::toml! {
                [sudo]

Review comment (suggestion):
To fix compilation:
```suggestion
            .parse(None, Some(context))
```

Code after:

    if config.disabled {
        return None;
    }

    if !config.allow_windows && env::consts::FAMILY == "windows" {
        return None;
    }

    let is_sudo_cached = context.exec_cmd("sudo", &["-n", "true"]).is_some();

    if !is_sudo_cached {
        return None;
    }

    let parsed = StringFormatter::new(config.format).and_then(|formatter| {
        formatter
            .map_meta(|variable, _| match variable {
                "symbol" => Some(config.symbol),
                _ => None,
            })
            .map_style(|variable| match variable {
                "style" => Some(Ok(config.style)),
                _ => None,
            })
            .parse(None, Some(context))
    });

    module.set_segments(match parsed {
        Ok(segments) => segments,
        Err(error) => {
            log::warn!("Error in module `sudo`:\n{}", error);
            return None;
        }
    });

    Some(module)
}

#[cfg(test)]
mod tests {
    use crate::{test::ModuleRenderer, utils::CommandOutput};
    use ansi_term::Color;

    #[test]
    fn test_sudo_not_cached() {
        let actual = ModuleRenderer::new("sudo")
            .cmd("sudo -n true", None)
            .config(toml::toml! {
                [sudo]
                disabled = false
Repository: starship/starship
File: src/modules/sudo.rs (Rust)

Code before:
            })
            .map_style(|variable| match variable {
                "style" => Some(Ok(config.style)),
                _ => None,
            })
            .parse(None)
    });

    module.set_segments(match parsed {
        Ok(segments) => segments,
        Err(error) => {
            log::warn!("Error in module `sudo`:\n{}", error);
            return None;
        }
    });

    Some(module)
}

#[cfg(test)]
mod tests {
    use crate::{test::ModuleRenderer, utils::CommandOutput};
    use ansi_term::Color;

    #[test]
    #[cfg(not(windows))]
    fn test_sudo_not_cached() {
        let actual = ModuleRenderer::new("sudo")
            .cmd("sudo -n true", None)
            .config(toml::toml! {
                [sudo]
                disabled = false
                allow_windows = true
            })
            .collect();
        let expected = None;

        assert_eq!(expected, actual);
    }

    #[test]
    #[cfg(not(windows))]
    fn test_sudo_cached() {
        let actual = ModuleRenderer::new("sudo")
            .cmd(
                "sudo -n true",
                Some(CommandOutput {
                    stdout: "".to_owned(),
                    stderr: "".to_owned(),
                }),
            )

Review comment (suggestion):
This test can run on Windows with the `allow_windows = true` change.

```suggestion
```

Code after:
            })
            .map_style(|variable| match variable {
                "style" => Some(Ok(config.style)),
                _ => None,
            })
            .parse(None, Some(context))
    });

    module.set_segments(match parsed {
        Ok(segments) => segments,
        Err(error) => {
            log::warn!("Error in module `sudo`:\n{}", error);
            return None;
        }
    });

    Some(module)
}

#[cfg(test)]
mod tests {
    use crate::{test::ModuleRenderer, utils::CommandOutput};
    use ansi_term::Color;

    #[test]
    fn test_sudo_not_cached() {
        let actual = ModuleRenderer::new("sudo")
            .cmd("sudo -n true", None)
            .config(toml::toml! {
                [sudo]
                disabled = false
                allow_windows = true
            })
            .collect();
        let expected = None;

        assert_eq!(expected, actual);
    }

    #[test]
    fn test_sudo_cached() {
        let actual = ModuleRenderer::new("sudo")
            .cmd(
                "sudo -n true",
                Some(CommandOutput {
                    stdout: "".to_owned(),
                    stderr: "".to_owned(),
                }),
            )
            .config(toml::toml! {
                [sudo]
Repository: starship/starship
File: src/init/mod.rs (Rust)

Code before:

const CMD_INIT: &str = include_str!("starship.lua");

#[cfg(test)]
mod tests {
    use super::*;
    #[test]
    fn escape_pwsh() -> io::Result<()> {
        let starship_path = StarshipPath {
            native_path: PathBuf::from(r"C:\starship.exe"),
        };
        assert_eq!(starship_path.sprint_pwsh()?, r"'C:\starship.exe'");
        Ok(())
    }

    #[test]
    fn escape_tick_pwsh() -> io::Result<()> {
        let starship_path = StarshipPath {
            native_path: PathBuf::from(r"C:\'starship.exe"),
        };
        assert_eq!(starship_path.sprint_pwsh()?, r"'C:\''starship.exe'");
        Ok(())
    }

    #[test]
    fn escape_cmd() -> io::Result<()> {
        let starship_path = StarshipPath {
            native_path: PathBuf::from(r"C:\starship.exe"),
        };
        assert_eq!(starship_path.sprint_cmd()?, r#""C:\starship.exe""#);
        Ok(())
    }

    #[test]
    fn escape_space_cmd() -> io::Result<()> {
        let starship_path = StarshipPath {
            native_path: PathBuf::from(r"C:\Cool Tools\starship.exe"),
        };
        assert_eq!(
            starship_path.sprint_cmd()?,
            r#""C:\Cool Tools\starship.exe""#
        );
        Ok(())
    }
}


Review comment (security):
Similar name changes here (`escape_cmd` sounds like we're testing escaping inputs to an external command).

Code after:

const CMDEXE_INIT: &str = include_str!("starship.lua");

#[cfg(test)]
mod tests {
    use super::*;
    #[test]
    fn escape_pwsh() -> io::Result<()> {
        let starship_path = StarshipPath {
            native_path: PathBuf::from(r"C:\starship.exe"),
        };
        assert_eq!(starship_path.sprint_pwsh()?, r"'C:\starship.exe'");
        Ok(())
    }

    #[test]
    fn escape_tick_pwsh() -> io::Result<()> {
        let starship_path = StarshipPath {
            native_path: PathBuf::from(r"C:\'starship.exe"),
        };
        assert_eq!(starship_path.sprint_pwsh()?, r"'C:\''starship.exe'");
        Ok(())
    }

    #[test]
    fn escape_cmdexe() -> io::Result<()> {
        let starship_path = StarshipPath {
            native_path: PathBuf::from(r"C:\starship.exe"),
        };
        assert_eq!(starship_path.sprint_cmdexe()?, r#""C:\starship.exe""#);
        Ok(())
    }

    #[test]
    fn escape_space_cmdexe() -> io::Result<()> {
        let starship_path = StarshipPath {
            native_path: PathBuf::from(r"C:\Cool Tools\starship.exe"),
        };
        assert_eq!(
            starship_path.sprint_cmdexe()?,
            r#""C:\Cool Tools\starship.exe""#
        );
        Ok(())
    }
}
Repository: starship/starship
File: src/modules/custom.rs (Rust)

Code before:
        None
    }
}

/// If the specified shell refers to PowerShell, adds the arguments "-Command -" to the
/// given command.
fn handle_powershell(command: &mut Command, shell: &str, shell_args: &[&str]) {
    let is_powershell = shell.ends_with("pwsh.exe")
        || shell.ends_with("powershell.exe")
        || shell.ends_with("pwsh")
        || shell.ends_with("powershell");

    if is_powershell && shell_args.is_empty() {
        command.arg("-NoProfile").arg("-Command").arg("-");
    }
}

#[cfg(windows)]
fn handle_cmdexe(command: &mut Command, shell: &str, shell_args: &[&str]) {
    let is_cmd = shell.ends_with("cmd.exe") || shell.ends_with("cmd");

    if is_cmd && shell_args.is_empty() {
        command.arg("/C");
    }
}

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

    #[cfg(not(windows))]
    const SHELL: &[&str] = &["/bin/sh"];
    #[cfg(windows)]
    const SHELL: &[&str] = &[];

    #[cfg(not(windows))]
    const FAILING_COMMAND: &str = "false";
    #[cfg(windows)]
    const FAILING_COMMAND: &str = "color 00";

    const UNKNOWN_COMMAND: &str = "ydelsyiedsieudleylse dyesdesl";

    #[test]
    fn when_returns_right_value() {
        assert!(exec_when("echo hello", SHELL));
        assert!(!exec_when(FAILING_COMMAND, SHELL));
    }

    #[test]
    fn when_returns_false_if_invalid_command() {
        assert!(!exec_when(UNKNOWN_COMMAND, SHELL));

Review comment (suggestion):
I just tried this, and it doesn't seem to work with a simple custom module of just `echo test`. I think the issue is that the command gets passed via stdin. See also the discussion in #2624 and #2629.

Code after:
        }

        Some(String::from_utf8_lossy(&output.stdout).into())
    } else {
        None
    }
}

/// If the specified shell refers to PowerShell, adds the arguments "-Command -" to the
/// given command.
fn handle_powershell(command: &mut Command, shell: &str, shell_args: &[&str]) {
    let is_powershell = shell.ends_with("pwsh.exe")
        || shell.ends_with("powershell.exe")
        || shell.ends_with("pwsh")
        || shell.ends_with("powershell");

    if is_powershell && shell_args.is_empty() {
        command.arg("-NoProfile").arg("-Command").arg("-");
    }
}

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

    #[cfg(not(windows))]
    const SHELL: &[&str] = &["/bin/sh"];
    #[cfg(windows)]
    const SHELL: &[&str] = &[];

    #[cfg(not(windows))]
    const FAILING_COMMAND: &str = "false";
    #[cfg(windows)]
    const FAILING_COMMAND: &str = "color 00";

    const UNKNOWN_COMMAND: &str = "ydelsyiedsieudleylse dyesdesl";

    #[test]
    fn when_returns_right_value() {
        assert!(exec_when("echo hello", SHELL));
        assert!(!exec_when(FAILING_COMMAND, SHELL));
    }

    #[test]
    fn when_returns_false_if_invalid_command() {
        assert!(!exec_when(UNKNOWN_COMMAND, SHELL));
    }

    #[test]
    #[cfg(not(windows))]
    fn command_returns_right_string() {
Repository: starship/starship
File: src/configs/starship_root.rs (Rust)

Code before:
use crate::{config::ModuleConfig, module::ALL_MODULES};

use serde::Serialize;
use std::cmp::Ordering;

// On changes please also update the `FullConfig` struct in `mod.rs`
#[derive(Clone, Serialize)]
pub struct StarshipRootConfig {
    pub format: String,
    pub right_format: String,
    pub scan_timeout: u64,
    pub command_timeout: u64,
    pub add_newline: bool,
}

// List of default prompt order
// NOTE: If this const value is changed then Default prompt order subheading inside
// prompt heading of config docs needs to be updated according to changes made here.
pub const PROMPT_ORDER: &[&str] = &[
    "username",
    "hostname",
    "localipv4",
    "shlvl",
    "singularity",
    "kubernetes",
    "directory",
    "vcsh",
    "git_branch",
    "git_commit",
    "git_state",
    "git_metrics",
    "git_status",
    "hg_branch",
    "docker_context",
    "package",
    // ↓ Toolchain version modules ↓
    // (Let's keep these sorted alphabetically)
    "cmake",
    "cobol",
    "dart",
    "deno",
    "dotnet",
    "elixir",
    "elm",
    "erlang",
    "golang",
    "helm",

Review comment (suggestion):
Unclear to me whether this should be `localip` or not (but I think it should?)

Code after:
use crate::{config::ModuleConfig, module::ALL_MODULES};

use serde::Serialize;
use std::cmp::Ordering;

// On changes please also update the `FullConfig` struct in `mod.rs`
#[derive(Clone, Serialize)]
pub struct StarshipRootConfig {
    pub format: String,
    pub right_format: String,
    pub continuation_prompt: String,
    pub scan_timeout: u64,
    pub command_timeout: u64,
    pub add_newline: bool,
}

// List of default prompt order
// NOTE: If this const value is changed then Default prompt order subheading inside
// prompt heading of config docs needs to be updated according to changes made here.
pub const PROMPT_ORDER: &[&str] = &[
    "username",
    "hostname",
    "localip",
    "shlvl",
    "singularity",
    "kubernetes",
    "directory",
    "vcsh",
    "git_branch",
    "git_commit",
    "git_state",
    "git_metrics",
    "git_status",
    "hg_branch",
    "docker_context",
    "package",
    // ↓ Toolchain version modules ↓
    // (Let's keep these sorted alphabetically)
    "cmake",
    "cobol",
    "dart",
    "deno",
    "dotnet",
    "elixir",
    "elm",
    "erlang",
    "golang",
Repository: starship/starship
File: src/init/starship.ps1 (PowerShell)

Code before:
#!/usr/bin/env pwsh

$null = New-Module starship {
    function Get-Arguments {
        function Get-Cwd {
            $cwd = Get-Location
            $provider_prefix = "$($cwd.Provider.ModuleName)\$($cwd.Provider.Name)::"
            return @{
                # Resolve the actual/physical path
                # NOTE: ProviderPath is only a physical filesystem path for the "FileSystem" provider
                # E.g. `Dev:\` -> `C:\Users\Joe Bloggs\Dev\`
                Path = $cwd.ProviderPath;
                # Resolve the provider-logical path
                # NOTE: Attempt to trim any "provider prefix" from the path string.
                # E.g. `Microsoft.PowerShell.Core\FileSystem::Dev:\` -> `Dev:\`
                LogicalPath =
                    if ($cwd.Path.StartsWith($provider_prefix)) {
                        $cwd.Path.Substring($provider_prefix.Length)
                    } else {
                        $cwd.Path
                    };
            }
        }

        # @ makes sure the result is an array even if single or no values are returned
        $jobs = @(Get-Job | Where-Object { $_.State -eq 'Running' }).Count

        $cwd = Get-Cwd
        $arguments = @(

Review comment (suggestion):
Please inline `Get-Arguments` again (`--continuation` doesn't need it after all, and it makes the whole thing a bit more readable).

Code after:
#!/usr/bin/env pwsh

# Create a new dynamic module so we don't pollute the global namespace with our functions and
# variables
$null = New-Module starship {
    function Get-Cwd {
        $cwd = Get-Location
        $provider_prefix = "$($cwd.Provider.ModuleName)\$($cwd.Provider.Name)::"
        return @{
            # Resolve the actual/physical path
            # NOTE: ProviderPath is only a physical filesystem path for the "FileSystem" provider
            # E.g. `Dev:\` -> `C:\Users\Joe Bloggs\Dev\`
            Path = $cwd.ProviderPath;
            # Resolve the provider-logical path
            # NOTE: Attempt to trim any "provider prefix" from the path string.
            # E.g. `Microsoft.PowerShell.Core\FileSystem::Dev:\` -> `Dev:\`
            LogicalPath =
                if ($cwd.Path.StartsWith($provider_prefix)) {
                    $cwd.Path.Substring($provider_prefix.Length)
                } else {
                    $cwd.Path
                };
        }
    }

    function Invoke-Native {
        param($Executable, $Arguments)
        $startInfo = New-Object System.Diagnostics.ProcessStartInfo -ArgumentList $Executable -Property @{
            StandardOutputEncoding = [System.Text.Encoding]::UTF8;