ronantakizawa/codereview-bench huggingface dataset:
Repository: astral-sh/uv
File: crates/uv/src/commands/project/init.rs (Rust)

Code before:
        Ok(())
    }
}

#[derive(Debug)]
enum Author {
    Name(String),
    Email(String),
    NameEmail { name: String, email: String },
}

impl Author {
    fn to_toml_string(&self) -> String {
        let mut inline = InlineTable::new();

        match self {
            Self::NameEmail { name, email } => {
                inline.get_or_insert("name", name);
                inline.get_or_insert("email", email);
            }
            Self::Name(name) => {
                inline.get_or_insert("name", name);
            }
            Self::Email(email) => {
                inline.get_or_insert("email", email);
            }
        }

        inline.to_string()
    }
}

/// Generate the `[project]` section of a `pyproject.toml`.
fn pyproject_project(
    name: &PackageName,
    requires_python: &RequiresPython,
    author: Option<&Author>,
    description: Option<&str>,
    no_description: bool,
    no_readme: bool,
) -> String {
    indoc::formatdoc! {r#"
        [project]
        name = "{name}"
        version = "0.1.0"{description}{readme}{authors}
        requires-python = "{requires_python}"
        dependencies = []
    "#,
        readme = if no_readme { "" } else { "\nreadme = \"README.md\"" },
        description = if no_description {
            String::new()

Review comment (suggestion):
Curious: do we want `get_or_insert` here, or just `insert`? To my understanding we're building the `pyproject.toml` from complete scratch, so there's no "get" path to take anyways.

Code after:
        Ok(())
    }
}

#[derive(Debug)]
enum Author {
    Name(String),
    Email(String),
    NameEmail { name: String, email: String },
}

impl Author {
    fn to_toml_string(&self) -> String {
        let mut inline = InlineTable::new();

        match self {
            Self::NameEmail { name, email } => {
                inline.insert("name", Value::from(name));
                inline.insert("email", Value::from(email));
            }
            Self::Name(name) => {
                inline.insert("name", Value::from(name));
            }
            Self::Email(email) => {
                inline.insert("email", Value::from(email));
            }
        }

        inline.to_string()
    }
}

/// Generate the `[project]` section of a `pyproject.toml`.
fn pyproject_project(
    name: &PackageName,
    requires_python: &RequiresPython,
    author: Option<&Author>,
    description: Option<&str>,
    no_description: bool,
    no_readme: bool,
) -> String {
    indoc::formatdoc! {r#"
        [project]
        name = "{name}"
        version = "0.1.0"{description}{readme}{authors}
        requires-python = "{requires_python}"
        dependencies = []
    "#,
        readme = if no_readme { "" } else { "\nreadme = \"README.md\"" },
        description = if no_description {
            String::new()
Repository: astral-sh/uv
File: crates/uv/tests/it/auth.rs (Rust)

Code before:
    ));

    // Store credentials with specific username
    uv_snapshot!(context.filters(), context.auth_login()
        .arg("https://test.example.com")
        .arg("--username").arg("specificuser")
        .arg("--password").arg("specificpass"), @r###"
    success: true
    exit_code: 0
    ----- stdout -----

    ----- stderr -----
    Stored credentials for specificuser@https://test.example.com/
    "###);

    // Test with username in URI
    uv_snapshot!(context.filters(), context.auth_helper(),
        input=r#"{"uri":"https://specificuser@test.example.com/path"}"#,
        @r#"
    success: true
    exit_code: 0
    ----- stdout -----
    {"headers":{"Authorization":["Basic [REDACTED]"]}}

    ----- stderr -----
    "#
    );
}


Review comment (security):
nit: we can add a test case with `differentuser` that checks that a different user doesn't get any credentials, i.e. we're using the user field.

Code after:
    );
}

/// Test credential helper with username in URI
#[test]
fn bazel_helper_username_in_uri() {
    let context = TestContext::new("3.12");

    // Store credentials with specific username
    uv_snapshot!(context.filters(), context.auth_login()
        .arg("https://test.example.com")
        .arg("--username").arg("specificuser")
        .arg("--password").arg("specificpass"), @r###"
    success: true
    exit_code: 0
    ----- stdout -----

    ----- stderr -----
    Stored credentials for specificuser@https://test.example.com/
    "###);

    // Test with username in URI
    uv_snapshot!(context.filters(), context.auth_helper()
        .arg("--protocol=bazel")
        .arg("get"),
        input=r#"{"uri":"https://specificuser@test.example.com/path"}"#,
        @r#"
    success: true
    exit_code: 0
    ----- stdout -----
    {"headers":{"Authorization":["Basic c3BlY2lmaWN1c2VyOnNwZWNpZmljcGFzcw=="]}}

    ----- stderr -----
    warning: The `uv auth helper` command is experimental and may change without warning. Pass `--preview-features auth-helper` to disable this warning
    "#
    );
}

/// Test credential helper with unknown username in URI
#[test]
fn bazel_helper_unknown_username_in_uri() {
    let context = TestContext::new("3.12");

    // Store credentials with specific username
    uv_snapshot!(context.filters(), context.auth_login()
        .arg("https://test.example.com")
        .arg("--username").arg("specificuser")
        .arg("--password").arg("specificpass"), @r###"
    success: true
    exit_code: 0
    ----- stdout -----
Repository: astral-sh/uv
File: crates/uv/src/commands/auth/helper.rs (Rust)

Code before:
/// - Input (stdin): `{"uri": "https://example.com/path"}`
/// - Output (stdout): `{"headers": {"Authorization": ["Basic ..."]}}` or `{"headers": {}}`
/// - Errors: Written to stderr with non-zero exit code
///
/// Full spec is [available here](https://github.com/bazelbuild/proposals/blob/main/designs/2022-06-07-bazel-credential-helpers.md)
pub(crate) async fn helper(
    preview: Preview,
    network_settings: &NetworkSettings,
    printer: Printer,
) -> Result<ExitStatus> {
    let request = BazelCredentialRequest::from_stdin()?;

    // TODO: make this logic generic over the protocol by providing `request.uri` from a
    // trait - that should help with adding new protocols
    let url = Url::parse(&request.uri).context("Invalid URI in credential request")?;
    let safe_url = DisplaySafeUrl::from_url(url);

    let credentials = credentials_for_url(&safe_url, preview, network_settings).await?;

    let response = serde_json::to_string(
        &credentials
            .map(BazelCredentialResponse::try_from)
            .unwrap_or_else(|| Ok(BazelCredentialResponse::default()))?,
    )
    .context("Failed to serialize response as JSON")?;
    writeln!(printer.stdout(), "{response}")?;
    Ok(ExitStatus::Success)
}


Review comment (suggestion):
To print the response with `--quiet` too:

```suggestion
    writeln!(printer.stdout_important(), "{response}")?;
```

Arguably `printer.stdout()` is more consistent as most other APIs use it. But then again `--quiet` doesn't really fit with a machine-readable command.

Code after:
/// - Input (stdin): `{"uri": "https://example.com/path"}`
/// - Output (stdout): `{"headers": {"Authorization": ["Basic ..."]}}` or `{"headers": {}}`
/// - Errors: Written to stderr with non-zero exit code
///
/// Full spec is [available here](https://github.com/bazelbuild/proposals/blob/main/designs/2022-06-07-bazel-credential-helpers.md)
pub(crate) async fn helper(
    preview: Preview,
    network_settings: &NetworkSettings,
    printer: Printer,
) -> Result<ExitStatus> {
    if !preview.is_enabled(PreviewFeatures::AUTH_HELPER) {
        warn_user!(
            "The `uv auth helper` command is experimental and may change without warning. Pass `--preview-features {}` to disable this warning",
            PreviewFeatures::AUTH_HELPER
        );
    }

    let request = BazelCredentialRequest::from_stdin()?;

    // TODO: make this logic generic over the protocol by providing `request.uri` from a
    // trait - that should help with adding new protocols
    let credentials = credentials_for_url(&request.uri, preview, network_settings).await?;

    let response = serde_json::to_string(
        &credentials
            .map(BazelCredentialResponse::try_from)
            .unwrap_or_else(|| Ok(BazelCredentialResponse::default()))?,
    )
    .context("Failed to serialize response as JSON")?;
    writeln!(printer.stdout_important(), "{response}")?;
    Ok(ExitStatus::Success)
}
Repository: astral-sh/uv
File: crates/uv/tests/it/pip_debug.rs (Rust)

Code before:
use crate::common::{TestContext, uv_snapshot};

#[test]
fn debug_warn() {
    let context = TestContext::new("3.12");

    uv_snapshot!(context.pip_debug(), @r"
    success: false
    exit_code: 2
    ----- stdout -----

    ----- stderr -----
    warning: The `debug` subcommand is not present in `uv pip`
    "
    );
}


Review comment (bug):
We might want to match the message of the pip compat args?

https://github.com/astral-sh/uv/blob/2df06ebfbc2d4df90d47ef9a9e631ba9926712e0/crates/uv-cli/src/compat.rs#L338

so

> pip's `debug` command is unsupported

Code after:
use crate::common::{TestContext, uv_snapshot};

#[test]
fn debug_warn() {
    let context = TestContext::new("3.12");

    uv_snapshot!(context.pip_debug(), @r"
    success: false
    exit_code: 2
    ----- stdout -----

    ----- stderr -----
    error: pip's `debug` is unsupported (consider using `uvx pip debug` instead)
    "
    );
}
Repository: astral-sh/uv
File: crates/uv/src/commands/cache_size.rs (Rust)

Code before:
    threads: Option<usize>,
    printer: Printer,
    preview: Preview,
) -> Result<ExitStatus> {
    if !preview.is_enabled(PreviewFeatures::CACHE_SIZE) {
        warn_user!(
            "`uv cache size` is experimental and may change without warning. Pass `--preview-features {}` to disable this warning.",
            PreviewFeatures::CACHE_SIZE
        );
    }

    if !cache.root().exists() {
        if human_readable {
            writeln!(printer.stdout_important(), "0B")?;
        } else {
            writeln!(printer.stdout_important(), "0")?;
        }
        return Ok(ExitStatus::Success);
    }

    let mut disk_usage = DiskUsage::new(vec![cache.root().to_path_buf()]);

    if let Some(n) = threads {
        tracing::info!("Using {} threads to calculate cache size", n);
        disk_usage = disk_usage.num_workers(n);
    }

    let total_bytes = disk_usage.count_ignoring_errors();

    if human_readable {
        let (bytes, unit) = human_readable_bytes(total_bytes);
        writeln!(printer.stdout_important(), "{bytes:.1}{unit}")?;
    } else {
        writeln!(printer.stdout_important(), "{total_bytes}")?;
    }

    Ok(ExitStatus::Success)
}


Review comment (performance):
```suggestion
    let disk_usage = if let Some(n) = threads {
        tracing::info!("Using {} threads to calculate cache size", n);
        DiskUsage::new(vec![cache.root().to_path_buf()]).num_workers(n)
    } else {
        DiskUsage::new(vec![cache.root().to_path_buf()])
    };
```

nit: avoid the mut

Code after:
    printer: Printer,
    preview: Preview,
) -> Result<ExitStatus> {
    if !preview.is_enabled(PreviewFeatures::CACHE_SIZE) {
        warn_user!(
            "`uv cache size` is experimental and may change without warning. Pass `--preview-features {}` to disable this warning.",
            PreviewFeatures::CACHE_SIZE
        );
    }

    if !cache.root().exists() {
        if human_readable {
            writeln!(printer.stdout_important(), "0B")?;
        } else {
            writeln!(printer.stdout_important(), "0")?;
        }
        return Ok(ExitStatus::Success);
    }

    let disk_usage = DiskUsage::new(vec![cache.root().to_path_buf()]);

    let total_bytes = disk_usage.count_ignoring_errors();

    if human_readable {
        let (bytes, unit) = human_readable_bytes(total_bytes);
        writeln!(printer.stdout_important(), "{bytes:.1}{unit}")?;
    } else {
        writeln!(printer.stdout_important(), "{total_bytes}")?;
    }

    Ok(ExitStatus::Success)
}
Repository: astral-sh/uv
File: crates/uv/tests/it/pip_freeze.rs (Rust)

Code before:
    // Without --prefix, the packages should not be visible.
    uv_snapshot!(context.pip_freeze(), @r###"
    success: true
    exit_code: 0
    ----- stdout -----

    ----- stderr -----
    "###
    );

    Ok(())
}

#[test]
fn freeze_exclude() -> Result<()> {
    let context = TestContext::new("3.12");

    let requirements_txt = context.temp_dir.child("requirements.txt");
    requirements_txt.write_str("MarkupSafe==2.1.3\ntomli==2.0.1")?;

    // Run `pip sync`.
    context
        .pip_sync()
        .arg(requirements_txt.path())
        .assert()
        .success();

    // Run `pip freeze --exclude MarkupSafe`.
    uv_snapshot!(context.filters(), context.pip_freeze().arg("--exclude").arg("MarkupSafe"), @r###"
    success: true
    exit_code: 0
    ----- stdout -----
    tomli==2.0.1

    ----- stderr -----
    "###
    );

    // Run `pip freeze --exclude MarkupSafe --exclude tomli`.
    uv_snapshot!(context.filters(), context.pip_freeze().arg("--exclude").arg("MarkupSafe").arg("--exclude").arg("tomli"), @r###"
    success: true
    exit_code: 0
    ----- stdout -----

    ----- stderr -----
    "###
    );

    Ok(())
}


Review comment (suggestion):
I might just `pip install` these packages instead since it's simpler, and you don't need to pin versions because we use exclude-newer in the test suite.

Code after:
    // Without --prefix, the packages should not be visible.
    uv_snapshot!(context.pip_freeze(), @"
    success: true
    exit_code: 0
    ----- stdout -----

    ----- stderr -----
    "
    );

    Ok(())
}

#[test]
fn freeze_exclude() {
    let context = TestContext::new("3.12");

    let prefix = context.temp_dir.child("prefix");

    // Install packages to a prefix directory.
    context
        .pip_install()
        .arg("MarkupSafe")
        .arg("tomli")
        .arg("--prefix")
        .arg(prefix.path())
        .assert()
        .success();

    // Run `pip freeze --exclude MarkupSafe`.
    uv_snapshot!(context.filters(), context.pip_freeze().arg("--exclude").arg("MarkupSafe").arg("--prefix").arg(prefix.path()), @r###"
    success: true
    exit_code: 0
    ----- stdout -----
    tomli==2.0.1

    ----- stderr -----
    "###
    );

    // Run `pip freeze --exclude MarkupSafe --exclude tomli`.
    uv_snapshot!(context.filters(), context.pip_freeze().arg("--exclude").arg("MarkupSafe").arg("--exclude").arg("tomli").arg("--prefix").arg(prefix.path()), @r###"
    success: true
    exit_code: 0
    ----- stdout -----

    ----- stderr -----
    "###
    );
}
Repository: astral-sh/uv
File: crates/uv/tests/it/pip_freeze.rs (Rust)

Code before:
    uv_snapshot!(context.pip_freeze(), @r###"
    success: true
    exit_code: 0
    ----- stdout -----

    ----- stderr -----
    "###
    );

    Ok(())
}

#[test]
fn freeze_exclude() -> Result<()> {
    let context = TestContext::new("3.12");

    let requirements_txt = context.temp_dir.child("requirements.txt");
    requirements_txt.write_str("MarkupSafe==2.1.3\ntomli==2.0.1")?;

    let prefix = context.temp_dir.child("prefix");

    // Install packages to a prefix directory.
    context
        .pip_install()
        .arg("-r")
        .arg("requirements.txt")
        .arg("--prefix")
        .arg(prefix.path())
        .assert()
        .success();

    // Run `pip freeze --exclude MarkupSafe`.
    uv_snapshot!(context.filters(), context.pip_freeze().arg("--exclude").arg("MarkupSafe").arg("--prefix").arg(prefix.path()), @r###"
    success: true
    exit_code: 0
    ----- stdout -----
    tomli==2.0.1

    ----- stderr -----
    "###
    );

    // Run `pip freeze --exclude MarkupSafe --exclude tomli`.
    uv_snapshot!(context.filters(), context.pip_freeze().arg("--exclude").arg("MarkupSafe").arg("--exclude").arg("tomli").arg("--prefix").arg(prefix.path()), @r###"
    success: true
    exit_code: 0
    ----- stdout -----

    ----- stderr -----
    "###
    );

Review comment (suggestion):
I think you should drop the requirements.txt entirely and just use `MarkupSafe` and `ntomli` as arguments here.

Code after:
    uv_snapshot!(context.pip_freeze(), @"
    success: true
    exit_code: 0
    ----- stdout -----

    ----- stderr -----
    "
    );

    Ok(())
}

#[test]
fn freeze_exclude() {
    let context = TestContext::new("3.12");

    let prefix = context.temp_dir.child("prefix");

    // Install packages to a prefix directory.
    context
        .pip_install()
        .arg("MarkupSafe")
        .arg("tomli")
        .arg("--prefix")
        .arg(prefix.path())
        .assert()
        .success();

    // Run `pip freeze --exclude MarkupSafe`.
    uv_snapshot!(context.filters(), context.pip_freeze().arg("--exclude").arg("MarkupSafe").arg("--prefix").arg(prefix.path()), @r###"
    success: true
    exit_code: 0
    ----- stdout -----
    tomli==2.0.1

    ----- stderr -----
    "###
    );

    // Run `pip freeze --exclude MarkupSafe --exclude tomli`.
    uv_snapshot!(context.filters(), context.pip_freeze().arg("--exclude").arg("MarkupSafe").arg("--exclude").arg("tomli").arg("--prefix").arg(prefix.path()), @r###"
    success: true
    exit_code: 0
    ----- stdout -----

    ----- stderr -----
    "###
    );
}
Repository: astral-sh/uv
File: crates/uv-bin-install/src/lib.rs (Rust)

Code before:
    reporter: &dyn Reporter,
    platform_name: &str,
    format: ArchiveFormat,
    download_url: &Url,
    cache_entry: &CacheEntry,
) -> Result<PathBuf, Error> {
    let mut retry_state = RetryState::new(*retry_policy, download_url.clone());

    loop {
        let result = download_and_unpack(
            binary,
            version,
            client,
            cache,
            reporter,
            platform_name,
            format,
            download_url,
            cache_entry,
        )
        .await;

        let result = match result {
            Ok(path) => Ok(path),
            Err(err) => {
                if retry_state.should_retry(&err, err.attempts()).await {
                    continue;
                }

                if retry_state.total_retries() > 0 {
                    Err(Error::RetriedError {
                        err: Box::new(err),
                        retries: retry_state.total_retries(),
                    })
                } else {
                    Err(err)
                }
            }
        };
        return result;
    }
}

/// Download and unpackage a binary,
///
/// NOTE [`download_and_unpack_with_retry`] should be used instead.
async fn download_and_unpack(
    binary: Binary,
    version: &Version,
    client: &BaseClient,
    cache: &Cache,

Review comment (suggestion):
You modified `attempts` in another place to be `retries` and removed the +1. Is this use of `attempts` still accurate? In the original code, `1` is subtracted from this value.

Code after:
    cache: &Cache,
    reporter: &dyn Reporter,
    platform_name: &str,
    format: ArchiveFormat,
    download_url: &Url,
    cache_entry: &CacheEntry,
) -> Result<PathBuf, Error> {
    let mut retry_state = RetryState::start(*retry_policy, download_url.clone());

    loop {
        let result = download_and_unpack(
            binary,
            version,
            client,
            cache,
            reporter,
            platform_name,
            format,
            download_url,
            cache_entry,
        )
        .await;

        match result {
            Ok(path) => return Ok(path),
            Err(err) => {
                if let Some(backoff) = retry_state.should_retry(&err, err.retries()) {
                    retry_state.sleep_backoff(backoff).await;
                    continue;
                }
                return if retry_state.total_retries() > 0 {
                    Err(Error::RetriedError {
                        err: Box::new(err),
                        retries: retry_state.total_retries(),
                    })
                } else {
                    Err(err)
                };
            }
        }
    }
}

/// Download and unpackage a binary,
///
/// NOTE [`download_and_unpack_with_retry`] should be used instead.
async fn download_and_unpack(
    binary: Binary,
    version: &Version,
    client: &BaseClient,
    cache: &Cache,
Repository: astral-sh/uv
File: crates/uv-fs/src/locked_file.rs (Rust)

Code before:
        .and_then(|lock_timeout| lock_timeout.parse::<u64>().ok())
    {
        Duration::from_secs(lock_timeout)
    } else {
        warn!(
            "Could not parse value of {} as integer: {:?}",
            EnvVars::UV_LOCK_TIMEOUT,
            lock_timeout
        );
        default_timeout
    }
});

#[derive(Debug, Error)]
pub enum CreateLockedFileError {
    #[error("Could not create temporary file")]
    CreateTemporary(#[source] io::Error),
    #[error("Could not persist temporary file `{}`", path.user_display())]
    PersistTemporary {
        path: PathBuf,
        #[source]
        source: io::Error,
    },
    #[error("Could not open existing file")]
    OpenExisting(#[source] io::Error),
    #[error("Could not create or open the file")]
    CreateOrOpen(#[source] io::Error),
}

impl CreateLockedFileError {
    pub fn as_io_error(&self) -> &io::Error {
        match self {
            Self::CreateTemporary(err) => err,
            Self::PersistTemporary { source, .. } => source,
            Self::OpenExisting(err) => err,
            Self::CreateOrOpen(err) => err,
        }
    }
}

#[derive(Debug, Error)]
pub enum LockedFileError {
    #[error(
        "Timeout ({}s) when waiting for lock on `{}` at `{}`, is another uv process running? You can set `{}` to increase the timeout.",
        timeout.as_secs(),
        resource,
        path.user_display(),
        EnvVars::UV_LOCK_TIMEOUT
    )]
    Timeout {
        timeout: Duration,

Review comment (suggestion):
Does fs_err already say which one it is? If so, we can do `[error(transparent)]` to avoid repeating the message.

Code after:
        .to_str()
        .and_then(|lock_timeout| lock_timeout.parse::<u64>().ok())
    {
        Duration::from_secs(lock_timeout)
    } else {
        warn!(
            "Could not parse value of {} as integer: {:?}",
            EnvVars::UV_LOCK_TIMEOUT,
            lock_timeout
        );
        default_timeout
    }
});

#[derive(Debug, Error)]
pub enum LockedFileError {
    #[error(
        "Timeout ({}s) when waiting for lock on `{}` at `{}`, is another uv process running? You can set `{}` to increase the timeout.",
        timeout.as_secs(),
        resource,
        path.user_display(),
        EnvVars::UV_LOCK_TIMEOUT
    )]
    Timeout {
        timeout: Duration,
        resource: String,
        path: PathBuf,
    },
    #[error(
        "Could not acquire lock for `{}` at `{}`",
        resource,
        path.user_display()
    )]
    Lock {
        resource: String,
        path: PathBuf,
        #[source]
        source: io::Error,
    },
    #[error(transparent)]
    #[cfg(feature = "tokio")]
    JoinError(#[from] tokio::task::JoinError),
    #[error("Could not create temporary file")]
    CreateTemporary(#[source] io::Error),
    #[error("Could not persist temporary file `{}`", path.user_display())]
    PersistTemporary {
        path: PathBuf,
        #[source]
        source: io::Error,
    },
    #[error(transparent)]