ronantakizawa/codereview-bench huggingface dataset:
Repository: launchbadge/sqlx
File: tests/postgres/postgres.rs (Rust)

Code before:
        .fetch_one(&mut conn)
        .await
        .expect_err("Too small number should have failed");
    assert!(
        matches!(&too_small_error, sqlx::Error::Encode(_)),
        "expected encode error, got {too_small_error:?}"
    );

    let too_large_error = sqlx::query_scalar::<_, BigDecimal>("SELECT $1::numeric")
        .bind(&too_large)
        .fetch_one(&mut conn)
        .await
        .expect_err("Too large number should have failed");

    assert!(
        matches!(&too_large_error, sqlx::Error::Encode(_)),
        "expected encode error, got {too_large_error:?}",
    );
}

#[sqlx_macros::test]
async fn test_pg_copy_chunked() -> anyhow::Result<()> {
    let mut conn = new::<Postgres>().await?;

    // (1 GiB - 1) - command byte (1 byte) - length prefix (4 bytes)
    const COPY_MAX_DATA_LEN: usize = 1_073_741_824 - 1 - 1 - 4;

    let mut row = "1".repeat(COPY_MAX_DATA_LEN / 10 - 1);
    row.push_str("\n");

    // creates a payload with COPY_MAX_DATA_LEN + 1 as size
    let mut payload = row.repeat(10);
    payload.push_str("12345678\n");

    assert_eq!(payload.len(), COPY_MAX_DATA_LEN + 1);

    let mut copy = conn.copy_in_raw("COPY products(name) FROM STDIN").await?;

    assert!(copy.send(payload.as_bytes()).await.is_ok());
    assert!(copy.finish().await.is_ok());
    Ok(())
}


Review comment (suggestion):
Instead of repeating this constant for the test, it could just be exported from the `postgres` module but `#[doc(hidden)]`.

Code after:
        .fetch_one(&mut conn)
        .await
        .expect_err("Too small number should have failed");
    assert!(
        matches!(&too_small_error, sqlx::Error::Encode(_)),
        "expected encode error, got {too_small_error:?}"
    );

    let too_large_error = sqlx::query_scalar::<_, BigDecimal>("SELECT $1::numeric")
        .bind(&too_large)
        .fetch_one(&mut conn)
        .await
        .expect_err("Too large number should have failed");

    assert!(
        matches!(&too_large_error, sqlx::Error::Encode(_)),
        "expected encode error, got {too_large_error:?}",
    );
}

#[sqlx_macros::test]
async fn test_pg_copy_chunked() -> anyhow::Result<()> {
    let mut conn = new::<Postgres>().await?;

    let mut row = "1".repeat(PG_COPY_MAX_DATA_LEN / 10 - 1);
    row.push_str("\n");

    // creates a payload with COPY_MAX_DATA_LEN + 1 as size
    let mut payload = row.repeat(10);
    payload.push_str("12345678\n");

    assert_eq!(payload.len(), PG_COPY_MAX_DATA_LEN + 1);

    let mut copy = conn.copy_in_raw("COPY products(name) FROM STDIN").await?;

    assert!(copy.send(payload.as_bytes()).await.is_ok());
    assert!(copy.finish().await.is_ok());
    Ok(())
}
Repository: launchbadge/sqlx
File: sqlx-postgres/src/type_checking.rs (Rust)

Code before:

        #[cfg(all(feature = "chrono", not(feature = "time")))]
        sqlx::postgres::types::PgTimeTz<sqlx::types::chrono::NaiveTime, sqlx::types::chrono::FixedOffset>,

        #[cfg(feature = "time")]
        sqlx::types::time::Time,

        #[cfg(feature = "time")]
        sqlx::types::time::Date,

        #[cfg(feature = "time")]
        sqlx::types::time::PrimitiveDateTime,

        #[cfg(feature = "time")]
        sqlx::types::time::OffsetDateTime,

        #[cfg(feature = "time")]
        sqlx::postgres::types::PgTimeTz<sqlx::types::time::Time, sqlx::types::time::UtcOffset>,

        #[cfg(feature = "bigdecimal")]
        sqlx::types::BigDecimal,

        #[cfg(feature = "rust_decimal")]
        sqlx::types::Decimal,

        #[cfg(all(feature = "ipnet", not(feature = "ipnetwork")))]
        sqlx::types::ipnet::IpNet,

        #[cfg(feature = "ipnetwork")]
        sqlx::types::ipnetwork::IpNetwork,

        #[cfg(feature = "mac_address")]
        sqlx::types::mac_address::MacAddress,

        #[cfg(feature = "json")]
        sqlx::types::JsonValue,

        #[cfg(feature = "bit-vec")]
        sqlx::types::BitVec,

        sqlx::postgres::types::PgHstore,
        // Arrays

        Vec<bool> | &[bool],
        Vec<String> | &[String],
        Vec<Vec<u8>> | &[Vec<u8>],
        Vec<i8> | &[i8],
        Vec<i16> | &[i16],
        Vec<i32> | &[i32],
        Vec<i64> | &[i64],
        Vec<f32> | &[f32],

Review comment (suggestion):
This isn't necessary, just order these _after_ the types from `ipnetwork`.

Code after:

        #[cfg(all(feature = "chrono", not(feature = "time")))]
        sqlx::postgres::types::PgTimeTz<sqlx::types::chrono::NaiveTime, sqlx::types::chrono::FixedOffset>,

        #[cfg(feature = "time")]
        sqlx::types::time::Time,

        #[cfg(feature = "time")]
        sqlx::types::time::Date,

        #[cfg(feature = "time")]
        sqlx::types::time::PrimitiveDateTime,

        #[cfg(feature = "time")]
        sqlx::types::time::OffsetDateTime,

        #[cfg(feature = "time")]
        sqlx::postgres::types::PgTimeTz<sqlx::types::time::Time, sqlx::types::time::UtcOffset>,

        #[cfg(feature = "bigdecimal")]
        sqlx::types::BigDecimal,

        #[cfg(feature = "rust_decimal")]
        sqlx::types::Decimal,

        #[cfg(feature = "ipnetwork")]
        sqlx::types::ipnetwork::IpNetwork,

        #[cfg(feature = "ipnet")]
        sqlx::types::ipnet::IpNet,

        #[cfg(feature = "mac_address")]
        sqlx::types::mac_address::MacAddress,

        #[cfg(feature = "json")]
        sqlx::types::JsonValue,

        #[cfg(feature = "bit-vec")]
        sqlx::types::BitVec,

        sqlx::postgres::types::PgHstore,
        // Arrays

        Vec<bool> | &[bool],
        Vec<String> | &[String],
        Vec<Vec<u8>> | &[Vec<u8>],
        Vec<i8> | &[i8],
        Vec<i16> | &[i16],
        Vec<i32> | &[i32],
        Vec<i64> | &[i64],
        Vec<f32> | &[f32],
Repository: launchbadge/sqlx
File: sqlx-cli/src/opt.rs (Rust)

Code before:
use std::ops::{Deref, Not};

use clap::{Args, Parser};
#[cfg(feature = "completions")]
use clap_complete::Shell;

#[derive(Parser, Debug)]
#[clap(version, about, author)]
pub struct Opt {
    /// Do not automatically load `.env` files.
    #[clap(long, default_value = "false")]
    pub no_dotenv: bool,

    #[clap(subcommand)]
    pub command: Command,
}

#[derive(Parser, Debug)]
pub enum Command {
    #[clap(alias = "db")]
    Database(DatabaseOpt),

    /// Generate query metadata to support offline compile-time verification.
    ///
    /// Saves metadata for all invocations of `query!` and related macros to a `.sqlx` directory
    /// in the current directory (or workspace root with `--workspace`), overwriting if needed.
    ///
    /// During project compilation, the absence of the `DATABASE_URL` environment variable or
    /// the presence of `SQLX_OFFLINE` (with a value of `true` or `1`) will constrain the
    /// compile-time verification to only read from the cached query metadata.
    #[clap(alias = "prep")]
    Prepare {
        /// Run in 'check' mode. Exits with 0 if the query metadata is up-to-date. Exits with
        /// 1 if the query metadata needs updating.
        #[clap(long)]
        check: bool,

Review comment (suggestion):
```suggestion
    #[clap(long)]
```

`default_value = "false"` is redundant for a boolean flag.

Code after:
use std::ops::{Deref, Not};

use clap::{Args, Parser};
#[cfg(feature = "completions")]
use clap_complete::Shell;

#[derive(Parser, Debug)]
#[clap(version, about, author)]
pub struct Opt {
    /// Do not automatically load `.env` files.
    #[clap(long)]
    pub no_dotenv: bool,

    #[clap(subcommand)]
    pub command: Command,
}

#[derive(Parser, Debug)]
pub enum Command {
    #[clap(alias = "db")]
    Database(DatabaseOpt),

    /// Generate query metadata to support offline compile-time verification.
    ///
    /// Saves metadata for all invocations of `query!` and related macros to a `.sqlx` directory
    /// in the current directory (or workspace root with `--workspace`), overwriting if needed.
    ///
    /// During project compilation, the absence of the `DATABASE_URL` environment variable or
    /// the presence of `SQLX_OFFLINE` (with a value of `true` or `1`) will constrain the
    /// compile-time verification to only read from the cached query metadata.
    #[clap(alias = "prep")]
    Prepare {
        /// Run in 'check' mode. Exits with 0 if the query metadata is up-to-date. Exits with
        /// 1 if the query metadata needs updating.
        #[clap(long)]
        check: bool,
Repository: launchbadge/sqlx
File: sqlx-cli/src/bin/cargo-sqlx.rs (Rust)

Code before:
use clap::Parser;
use console::style;
use sqlx_cli::Opt;
use std::process;

// cargo invokes this binary as `cargo-sqlx sqlx <args>`
// so the parser below is defined with that in mind
#[derive(Parser, Debug)]
#[clap(bin_name = "cargo")]
enum Cli {
    Sqlx(Opt),
}

#[tokio::main]
async fn main() {
    dotenvy::dotenv().ok();
    let Cli::Sqlx(opt) = Cli::parse();

    if let Err(error) = sqlx_cli::run(opt).await {
        println!("{} {}", style("error:").bold().red(), error);
        process::exit(1);
    }
}


Review comment (suggestion):
Note: this PR actually regressed default handling of `.env` files because it moves the `dotenv()` call _after_ the Clap parsing, which reads `DATABASE_URL` from the environment.

Releasing a hotfix in #3824.

Code after:
use clap::Parser;
use console::style;
use sqlx_cli::Opt;
use std::process;

// cargo invokes this binary as `cargo-sqlx sqlx <args>`
// so the parser below is defined with that in mind
#[derive(Parser, Debug)]
#[clap(bin_name = "cargo")]
enum Cli {
    Sqlx(Opt),
}

#[tokio::main]
async fn main() {
    let Cli::Sqlx(opt) = Cli::parse();

    if !opt.no_dotenv {
        dotenvy::dotenv().ok();
    }

    if let Err(error) = sqlx_cli::run(opt).await {
        println!("{} {}", style("error:").bold().red(), error);
        process::exit(1);
    }
}
Repository: launchbadge/sqlx
File: sqlx-postgres/src/options/mod.rs (Rust)

Code before:
        }
    }

    // fallback to localhost if no socket was found
    "localhost".to_owned()
}

#[test]
fn test_options_formatting() {
    let options = PgConnectOptions::new().options([("geqo", "off")]);
    assert_eq!(options.options, Some("-c geqo=off".to_string()));
    let options = options.options([("search_path", "sqlx")]);
    assert_eq!(
        options.options,
        Some("-c geqo=off -c search_path=sqlx".to_string())
    );
    let options = PgConnectOptions::new().options([("geqo", "off"), ("statement_timeout", "5min")]);
    assert_eq!(
        options.options,
        Some("-c geqo=off -c statement_timeout=5min".to_string())
    );
    // https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-OPTIONS
    let options = PgConnectOptions::new().options([("passfile", "/back\\slash/ and\\ spaces")]);
    assert_eq!(
        options.options,
        Some("-c passfile=/back\\\\slash/\\ and\\\\\\ spaces".to_string())
    );
    let options = PgConnectOptions::new();
    assert_eq!(options.options, None);
}


Review comment (suggestion):
This can be made a lot nicer with [raw string literals](https://doc.rust-lang.org/reference/tokens.html#raw-string-literals):

```suggestion
    let options = PgConnectOptions::new().options([("passfile", r"/back\slash/ and\ spaces")]);
    assert_eq!(
        options.options,
        Some(r"-c passfile=/back\\slash/\ and\\\ spaces".to_string())
```



Code after:
}

/// Writer that escapes passed-in PostgreSQL options.
///
/// Escapes backslashes and spaces with an additional backslash according to
/// https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-OPTIONS
#[derive(Debug)]
struct PgOptionsWriteEscaped<'a>(&'a mut String);

impl Write for PgOptionsWriteEscaped<'_> {
    fn write_str(&mut self, s: &str) -> fmt::Result {
        let mut span_start = 0;

        for (span_end, matched) in s.match_indices([' ', '\\']) {
            write!(self.0, r"{}\{matched}", &s[span_start..span_end])?;
            span_start = span_end + matched.len();
        }

        // Write the rest of the string after the last match, or all of it if no matches
        self.0.push_str(&s[span_start..]);

        Ok(())
    }

    fn write_char(&mut self, ch: char) -> fmt::Result {
        if matches!(ch, ' ' | '\\') {
            self.0.push('\\');
        }

        self.0.push(ch);

        Ok(())
    }
}

#[test]
fn test_options_formatting() {
    let options = PgConnectOptions::new().options([("geqo", "off")]);
    assert_eq!(options.options, Some("-c geqo=off".to_string()));
    let options = options.options([("search_path", "sqlx")]);
    assert_eq!(
        options.options,
        Some("-c geqo=off -c search_path=sqlx".to_string())
    );
    let options = PgConnectOptions::new().options([("geqo", "off"), ("statement_timeout", "5min")]);
    assert_eq!(
        options.options,
        Some("-c geqo=off -c statement_timeout=5min".to_string())
    );
    // https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-OPTIONS
    let options =
Repository: launchbadge/sqlx
File: sqlx-macros-core/src/derives/attributes.rs (Rust)

Code before:
        );
    }

    Ok(attributes)
}

pub fn check_strong_enum_attributes(
    input: &DeriveInput,
    _variants: &Punctuated<Variant, Comma>,
) -> syn::Result<SqlxContainerAttributes> {
    let attributes = check_enum_attributes(input)?;

    assert_attribute!(attributes.repr.is_none(), "unexpected #[repr(..)]", input);

    Ok(attributes)
}

pub fn check_struct_attributes(
    input: &DeriveInput,
    fields: &Punctuated<Field, Comma>,
) -> syn::Result<SqlxContainerAttributes> {
    let attributes = parse_container_attributes(&input.attrs)?;

    assert_attribute!(
        !attributes.transparent,
        "unexpected #[sqlx(transparent)] - only valid for structs with exactly one field",
        input
    );

    assert_attribute!(
        attributes.rename_all.is_none(),
        "unexpected #[sqlx(rename_all = ..)]",
        input
    );

    assert_attribute!(attributes.repr.is_none(), "unexpected #[repr(..)]", input);

    for field in fields {
        let attributes = parse_child_attributes(&field.attrs)?;

        assert_attribute!(
            attributes.rename.is_none(),
            "unexpected #[sqlx(rename = ..)]",
            field
        );
    }

    Ok(attributes)
}


Review comment (suggestion):
```suggestion
        "#[sqlx(transparent)] is only valid for structs with exactly one field",
```

It's no longer unexpected.

Code after:
        );
    }

    Ok(attributes)
}

pub fn check_strong_enum_attributes(
    input: &DeriveInput,
    _variants: &Punctuated<Variant, Comma>,
) -> syn::Result<SqlxContainerAttributes> {
    let attributes = check_enum_attributes(input)?;

    assert_attribute!(attributes.repr.is_none(), "unexpected #[repr(..)]", input);

    Ok(attributes)
}

pub fn check_struct_attributes(
    input: &DeriveInput,
    fields: &Punctuated<Field, Comma>,
) -> syn::Result<SqlxContainerAttributes> {
    let attributes = parse_container_attributes(&input.attrs)?;

    assert_attribute!(
        !attributes.transparent,
        "#[sqlx(transparent)] is only valid for structs with exactly one field",
        input
    );

    assert_attribute!(
        attributes.rename_all.is_none(),
        "unexpected #[sqlx(rename_all = ..)]",
        input
    );

    assert_attribute!(attributes.repr.is_none(), "unexpected #[repr(..)]", input);

    for field in fields {
        let attributes = parse_child_attributes(&field.attrs)?;

        assert_attribute!(
            attributes.rename.is_none(),
            "unexpected #[sqlx(rename = ..)]",
            field
        );
    }

    Ok(attributes)
}
Repository: libsdl-org/SDL
File: src/process/windows/SDL_windowsprocess.c (C)

Code before:
    return true;
}

static bool is_batch_file_path(const char *path) {
    size_t len_path = SDL_strlen(path);
    if (len_path < 4) {
        return false;
    }
    if (SDL_strcasecmp(path + len_path - 4, ".bat") == 0 || SDL_strcasecmp(path + len_path - 4, ".cmd") == 0) {
        return true;
    }
    return false;
}

static bool join_arguments(const char * const *args, LPWSTR *args_out)
{
    size_t len;
    int i;
    size_t i_out;
    char *result;
    bool batch_file = is_batch_file_path(args[0]);

    len = 0;
    for (i = 0; args[i]; i++) {
        const char *a = args[i];
        bool quotes = *a == '\0' || strpbrk(a, " \r\n\t\v") != NULL;

        if (quotes) {
            /* surround the argument with double quote if it is empty or contains whitespaces */
            len += 2;
        }

        for (; *a; a++) {
            switch (*a) {
            case '"':
                len += 2;
                break;
            case '\\':
                /* only escape backslashes that precede a double quote (including the enclosing double quote) */
                len += (a[1] == '"' || (quotes && a[1] == '\0')) ? 2 : 1;
                break;
            case ' ':
            case '^':
            case '&':
            case '|':
            case '<':
            case '>':
                if (batch_file) {
                    len += 2;
                } else {
                    len += 1;

Review comment (suggestion):
```suggestion
        bool quotes = *a == '\0' || SDL_strpbrk(a, " \r\n\t\v") != NULL;
```

Code after:
    return true;
}

static bool is_batch_file_path(const char *path) {
    size_t len_path = SDL_strlen(path);
    if (len_path < 4) {
        return false;
    }
    if (SDL_strcasecmp(path + len_path - 4, ".bat") == 0 || SDL_strcasecmp(path + len_path - 4, ".cmd") == 0) {
        return true;
    }
    return false;
}

static bool join_arguments(const char * const *args, LPWSTR *args_out)
{
    size_t len;
    int i;
    size_t i_out;
    char *result;
    bool batch_file = is_batch_file_path(args[0]);

    len = 0;
    for (i = 0; args[i]; i++) {
        const char *a = args[i];
        bool quotes = *a == '\0' || SDL_strpbrk(a, " \r\n\t\v") != NULL;

        if (quotes) {
            /* surround the argument with double quote if it is empty or contains whitespaces */
            len += 2;
        }

        for (; *a; a++) {
            switch (*a) {
            case '"':
                len += 2;
                break;
            case '\\':
                /* only escape backslashes that precede a double quote (including the enclosing double quote) */
                len += (a[1] == '"' || (quotes && a[1] == '\0')) ? 2 : 1;
                break;
            case ' ':
            case '^':
            case '&':
            case '|':
            case '<':
            case '>':
                if (batch_file) {
                    len += 2;
                } else {
                    len += 1;
Repository: libsdl-org/SDL
File: src/process/windows/SDL_windowsprocess.c (C)

Code before:
            case '>':
                if (batch_file) {
                    len += 2;
                } else {
                    len += 1;
                }
                break;
            default:
                len += 1;
                break;
            }
        }
        /* space separator or final '\0' */
        len += 1;
    }

    result = SDL_malloc(len);
    if (!result) {
        *args_out = NULL;
        return false;
    }

    i_out = 0;
    for (i = 0; args[i]; i++) {
        const char *a = args[i];
        bool quotes = *a == '\0' || strpbrk(a, " \r\n\t\v") != NULL;

        if (quotes) {
            result[i_out++] = '"';
        }
        for (; *a; a++) {
            switch (*a) {
            case '"':
                if (batch_file) {
                    result[i_out++] = '"';
                } else {
                    result[i_out++] = '\\';
                }
                result[i_out++] = *a;
                break;
            case '\\':
                result[i_out++] = *a;
                if (a[1] == '"' || (quotes && a[1] == '\0')) {
                    result[i_out++] = *a;
                }
                break;
            case ' ':
                if (batch_file) {
                    result[i_out++] = '^';
                }
                result[i_out++] = *a;

Review comment (suggestion):
```suggestion
        bool quotes = *a == '\0' || SDL_strpbrk(a, " \r\n\t\v") != NULL;
```

Code after:
            case '>':
                if (batch_file) {
                    len += 2;
                } else {
                    len += 1;
                }
                break;
            default:
                len += 1;
                break;
            }
        }
        /* space separator or final '\0' */
        len += 1;
    }

    result = SDL_malloc(len);
    if (!result) {
        *args_out = NULL;
        return false;
    }

    i_out = 0;
    for (i = 0; args[i]; i++) {
        const char *a = args[i];
        bool quotes = *a == '\0' || SDL_strpbrk(a, " \r\n\t\v") != NULL;

        if (quotes) {
            result[i_out++] = '"';
        }
        for (; *a; a++) {
            switch (*a) {
            case '"':
                if (batch_file) {
                    result[i_out++] = '"';
                } else {
                    result[i_out++] = '\\';
                }
                result[i_out++] = *a;
                break;
            case '\\':
                result[i_out++] = *a;
                if (a[1] == '"' || (quotes && a[1] == '\0')) {
                    result[i_out++] = *a;
                }
                break;
            case ' ':
                if (batch_file) {
                    result[i_out++] = '^';
                }
                result[i_out++] = *a;