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

Code before:
            buf.encode(value)?;
            count += 1;
        }

        const OVERFLOW: usize = MAX + 1;
        if iter.next().is_some() {
            return Err(format!("encoded iterator is too large for Postgres: {OVERFLOW}").into());
        }

        // set the length now that we know what it is.
        buf[len_start..(len_start + 4)].copy_from_slice(count.to_be_bytes().as_slice());

        Ok(IsNull::No)
    }
}

impl<'q, T, I> Encode<'q, Postgres> for PgBindIter<I>
where
    T: Type<Postgres> + Encode<'q, Postgres>,
    // Clone is required for the encode_by_ref call since we can't iterate with a shared reference
    I: Iterator<Item = T> + Clone,
{
    fn encode_by_ref(
        &self,
        buf: &mut PgArgumentBuffer,
    ) -> Result<IsNull, Box<dyn std::error::Error + Send + Sync + 'static>> {
        Self::encode_inner(self.0.clone(), buf)
    }
    fn encode(
        self,
        buf: &mut PgArgumentBuffer,
    ) -> Result<IsNull, Box<dyn std::error::Error + Send + Sync + 'static>>
    where
        Self: Sized,
    {
        Self::encode_inner(self.0, buf)
    }
}


Review comment (suggestion):
```suggestion
    ) -> Result<IsNull, BoxDynError> {
```

Code after:

impl<'q, I> PgBindIter<I>
where
    I: Iterator,
    <I as Iterator>::Item: Type<Postgres> + Encode<'q, Postgres>,
{
    fn encode_inner(
        // need ownership to iterate
        mut iter: I,
        buf: &mut PgArgumentBuffer,
    ) -> Result<IsNull, BoxDynError> {
        let lower_size_hint = iter.size_hint().0;
        let first = iter.next();
        let type_info = first
            .as_ref()
            .and_then(Encode::produces)
            .unwrap_or_else(<I as Iterator>::Item::type_info);

        buf.extend(&1_i32.to_be_bytes()); // number of dimensions
        buf.extend(&0_i32.to_be_bytes()); // flags

        match type_info.0 {
            PgType::DeclareWithName(name) => buf.patch_type_by_name(&name),
            PgType::DeclareArrayOf(array) => buf.patch_array_type(array),

            ty => {
                buf.extend(&ty.oid().0.to_be_bytes());
            }
        }

        let len_start = buf.len();
        buf.extend(0_i32.to_be_bytes()); // len (unknown so far)
        buf.extend(1_i32.to_be_bytes()); // lower bound

        match first {
            Some(first) => buf.encode(first)?,
            None => return Ok(IsNull::No),
        }

        let mut count = 1_i32;
        const MAX: usize = i32::MAX as usize - 1;

        for value in (&mut iter).take(MAX) {
            buf.encode(value)?;
            count += 1;
        }

        const OVERFLOW: usize = i32::MAX as usize + 1;
        if iter.next().is_some() {
            let iter_size = std::cmp::max(lower_size_hint, OVERFLOW);
            return Err(format!("encoded iterator is too large for Postgres: {iter_size}").into());
Repository: launchbadge/sqlx
File: sqlx-postgres/src/bind_iter.rs (Rust)

Code before:
            return Err(format!("encoded iterator is too large for Postgres: {OVERFLOW}").into());
        }

        // set the length now that we know what it is.
        buf[len_start..(len_start + 4)].copy_from_slice(count.to_be_bytes().as_slice());

        Ok(IsNull::No)
    }
}

impl<'q, T, I> Encode<'q, Postgres> for PgBindIter<I>
where
    T: Type<Postgres> + Encode<'q, Postgres>,
    // Clone is required for the encode_by_ref call since we can't iterate with a shared reference
    I: Iterator<Item = T> + Clone,
{
    fn encode_by_ref(
        &self,
        buf: &mut PgArgumentBuffer,
    ) -> Result<IsNull, Box<dyn std::error::Error + Send + Sync + 'static>> {
        Self::encode_inner(self.0.clone(), buf)
    }
    fn encode(
        self,
        buf: &mut PgArgumentBuffer,
    ) -> Result<IsNull, Box<dyn std::error::Error + Send + Sync + 'static>>
    where
        Self: Sized,
    {
        Self::encode_inner(self.0, buf)
    }
}


Review comment (suggestion):
```suggestion
    ) -> Result<IsNull, BoxDynError>
```

Code after:
    fn encode_inner(
        // need ownership to iterate
        mut iter: I,
        buf: &mut PgArgumentBuffer,
    ) -> Result<IsNull, BoxDynError> {
        let lower_size_hint = iter.size_hint().0;
        let first = iter.next();
        let type_info = first
            .as_ref()
            .and_then(Encode::produces)
            .unwrap_or_else(<I as Iterator>::Item::type_info);

        buf.extend(&1_i32.to_be_bytes()); // number of dimensions
        buf.extend(&0_i32.to_be_bytes()); // flags

        match type_info.0 {
            PgType::DeclareWithName(name) => buf.patch_type_by_name(&name),
            PgType::DeclareArrayOf(array) => buf.patch_array_type(array),

            ty => {
                buf.extend(&ty.oid().0.to_be_bytes());
            }
        }

        let len_start = buf.len();
        buf.extend(0_i32.to_be_bytes()); // len (unknown so far)
        buf.extend(1_i32.to_be_bytes()); // lower bound

        match first {
            Some(first) => buf.encode(first)?,
            None => return Ok(IsNull::No),
        }

        let mut count = 1_i32;
        const MAX: usize = i32::MAX as usize - 1;

        for value in (&mut iter).take(MAX) {
            buf.encode(value)?;
            count += 1;
        }

        const OVERFLOW: usize = i32::MAX as usize + 1;
        if iter.next().is_some() {
            let iter_size = std::cmp::max(lower_size_hint, OVERFLOW);
            return Err(format!("encoded iterator is too large for Postgres: {iter_size}").into());
        }

        // set the length now that we know what it is.
        buf[len_start..(len_start + 4)].copy_from_slice(&count.to_be_bytes());

        Ok(IsNull::No)
Repository: launchbadge/sqlx
File: sqlx-postgres/src/types/mod.rs (Rust)

Code before:
//!
//! User-defined enumerations are supported through a derive for `Type`.
//!
//! ```text
//! CREATE TYPE mood AS ENUM ('sad', 'ok', 'happy');
//! ```
//!
//! ```rust,ignore
//! #[derive(sqlx::Type)]
//! #[sqlx(type_name = "mood", rename_all = "lowercase")]
//! enum Mood { Sad, Ok, Happy }
//! ```
//!
//! Rust enumerations may also be defined to be represented as an integer using `repr`.
//! The following type expects a SQL type of `INTEGER` or `INT4` and will convert to/from the
//! Rust enumeration.
//!
//! ```rust,ignore
//! #[derive(sqlx::Type)]
//! #[repr(i32)]
//! enum Mood { Sad = 0, Ok = 1, Happy = 2 }
//! ```
//!
//! Rust enumerations may also be defined to be represented as a string using `type_name = "text"`.
//! The following type expects a SQL type of `TEXT` and will convert to/from the Rust enumeration.
//!
//! ```rust,ignore
//! #[derive(sqlx::Type)]
//! #[sqlx(type_name = "text")]
//! enum Mood { Sad, Ok, Happy }
//! ```

use crate::type_info::PgTypeKind;
use crate::{PgTypeInfo, Postgres};

pub(crate) use sqlx_core::types::{Json, Type};

mod array;
mod bool;
mod bytes;
mod citext;
mod float;
mod hstore;
mod int;
mod interval;
mod lquery;
mod ltree;
// Not behind a Cargo feature because we require JSON in the driver implementation.
mod json;
mod money;
mod oid;

Review comment (suggestion):
This maybe should mention that an error can occur if attempting to decode a value not contained within the enum definition. You never know what people might assume it does otherwise.

Code after:
//!
//! User-defined enumerations are supported through a derive for `Type`.
//!
//! ```text
//! CREATE TYPE mood AS ENUM ('sad', 'ok', 'happy');
//! ```
//!
//! ```rust,ignore
//! #[derive(sqlx::Type)]
//! #[sqlx(type_name = "mood", rename_all = "lowercase")]
//! enum Mood { Sad, Ok, Happy }
//! ```
//!
//! Rust enumerations may also be defined to be represented as an integer using `repr`.
//! The following type expects a SQL type of `INTEGER` or `INT4` and will convert to/from the
//! Rust enumeration.
//!
//! ```rust,ignore
//! #[derive(sqlx::Type)]
//! #[repr(i32)]
//! enum Mood { Sad = 0, Ok = 1, Happy = 2 }
//! ```
//!
//! Rust enumerations may also be defined to be represented as a string using `type_name = "text"`.
//! The following type expects a SQL type of `TEXT` and will convert to/from the Rust enumeration.
//!
//! ```rust,ignore
//! #[derive(sqlx::Type)]
//! #[sqlx(type_name = "text")]
//! enum Mood { Sad, Ok, Happy }
//! ```
//!
//! Note that an error can occur if you attempt to decode a value not contained within the enum
//! definition.
//!

use crate::type_info::PgTypeKind;
use crate::{PgTypeInfo, Postgres};

pub(crate) use sqlx_core::types::{Json, Type};

mod array;
mod bool;
mod bytes;
mod citext;
mod float;
mod hstore;
mod int;
mod interval;
mod lquery;
mod ltree;
Repository: launchbadge/sqlx
File: sqlx-cli/src/lib.rs (Rust)

Code before:
use crate::opt::{Command, ConnectOpts, DatabaseCommand, MigrateCommand};

mod database;
mod metadata;
// mod migration;
// mod migrator;
#[cfg(feature = "completions")]
mod completions;
mod migrate;
mod opt;
mod prepare;

pub use crate::opt::Opt;

pub async fn run(opt: Opt) -> Result<()> {
    let ctrlc_fut = signal::ctrl_c();
    let do_run_fut = do_run(opt);

    select! {
        biased;
        _ = ctrlc_fut => {
            Ok(())
        },
        do_run_outcome = do_run_fut => {
            do_run_outcome
        }
    }
}

async fn do_run(opt: Opt) -> Result<()> {
    match opt.command {
        Command::Migrate(migrate) => match migrate.command {
            MigrateCommand::Add {
                source,
                description,
                reversible,
                sequential,
                timestamp,
            } => migrate::add(&source, &description, reversible, sequential, timestamp).await?,
            MigrateCommand::Run {
                source,
                dry_run,
                ignore_missing,
                connect_opts,
                target_version,
            } => {
                migrate::run(
                    &source,
                    &connect_opts,
                    dry_run,
                    *ignore_missing,

Review comment (suggestion):
Should mention why we need to explicitly handle Ctrl-C.

Code after:
use crate::opt::{Command, ConnectOpts, DatabaseCommand, MigrateCommand};

mod database;
mod metadata;
// mod migration;
// mod migrator;
#[cfg(feature = "completions")]
mod completions;
mod migrate;
mod opt;
mod prepare;

pub use crate::opt::Opt;

pub async fn run(opt: Opt) -> Result<()> {
    // This `select!` is here so that when the process receives a `SIGINT` (CTRL + C),
    // the futures currently running on this task get dropped before the program exits.
    // This is currently necessary for the consumers of the `dialoguer` crate to restore
    // the user's terminal if the process is interrupted while a dialog is being displayed.

    let ctrlc_fut = signal::ctrl_c();
    let do_run_fut = do_run(opt);

    select! {
        biased;
        _ = ctrlc_fut => {
            Ok(())
        },
        do_run_outcome = do_run_fut => {
            do_run_outcome
        }
    }
}

async fn do_run(opt: Opt) -> Result<()> {
    match opt.command {
        Command::Migrate(migrate) => match migrate.command {
            MigrateCommand::Add {
                source,
                description,
                reversible,
                sequential,
                timestamp,
            } => migrate::add(&source, &description, reversible, sequential, timestamp).await?,
            MigrateCommand::Run {
                source,
                dry_run,
                ignore_missing,
                connect_opts,
                target_version,
            } => {
Repository: launchbadge/sqlx
File: sqlx-core/src/encode.rs (Rust)

Code before:
                buf: &mut <DB as Database>::ArgumentBuffer<'q>,
            ) -> Result<IsNull, BoxDynError> {
                <&T as Encode<DB>>::encode(self, buf)
            }

            #[inline]
            fn produces(&self) -> Option<DB::TypeInfo> {
                (**self).produces()
            }

            #[inline]
            fn size_hint(&self) -> usize {
                (**self).size_hint()
            }
        }
    };
}

impl_encode_for_smartpointer!(Arc<T>);
impl_encode_for_smartpointer!(Box<T>);
impl_encode_for_smartpointer!(Rc<T>);

impl<'q, T, DB: Database> Encode<'q, DB> for Cow<'_, T>
where
    T: Encode<'q, DB>,
    T: ToOwned,
{
    #[inline]
    fn encode(self, buf: &mut <DB as Database>::ArgumentBuffer<'q>) -> Result<IsNull, BoxDynError> {
        <&T as Encode<DB>>::encode_by_ref(&self.as_ref(), buf)
    }

    #[inline]
    fn encode_by_ref(
        &self,
        buf: &mut <DB as Database>::ArgumentBuffer<'q>,
    ) -> Result<IsNull, BoxDynError> {
        <&T as Encode<DB>>::encode_by_ref(&self.as_ref(), buf)
    }

    #[inline]
    fn produces(&self) -> Option<DB::TypeInfo> {
        <&T as Encode<DB>>::produces(&self.as_ref())
    }

    #[inline]
    fn size_hint(&self) -> usize {
        <&T as Encode<DB>>::size_hint(&self.as_ref())
    }
}


Review comment (suggestion):
```suggestion
    T: Encode<'q, DB> + ToOwned,
```

Code after:
                buf: &mut <DB as Database>::ArgumentBuffer<'q>,
            ) -> Result<IsNull, BoxDynError> {
                <&T as Encode<DB>>::encode(self, buf)
            }

            #[inline]
            fn produces(&self) -> Option<DB::TypeInfo> {
                (**self).produces()
            }

            #[inline]
            fn size_hint(&self) -> usize {
                (**self).size_hint()
            }
        }
    };
}

impl_encode_for_smartpointer!(Arc<T>);
impl_encode_for_smartpointer!(Box<T>);
impl_encode_for_smartpointer!(Rc<T>);

impl<'q, T, DB: Database> Encode<'q, DB> for Cow<'q, T>
where
    T: Encode<'q, DB>,
    T: ToOwned,
{
    #[inline]
    fn encode(self, buf: &mut <DB as Database>::ArgumentBuffer<'q>) -> Result<IsNull, BoxDynError> {
        <&T as Encode<DB>>::encode_by_ref(&self.as_ref(), buf)
    }

    #[inline]
    fn encode_by_ref(
        &self,
        buf: &mut <DB as Database>::ArgumentBuffer<'q>,
    ) -> Result<IsNull, BoxDynError> {
        <&T as Encode<DB>>::encode_by_ref(&self.as_ref(), buf)
    }

    #[inline]
    fn produces(&self) -> Option<DB::TypeInfo> {
        <&T as Encode<DB>>::produces(&self.as_ref())
    }

    #[inline]
    fn size_hint(&self) -> usize {
        <&T as Encode<DB>>::size_hint(&self.as_ref())
    }
}
Repository: launchbadge/sqlx
File: sqlx-core/src/encode.rs (Rust)

Code before:
                &self,
                buf: &mut <DB as Database>::ArgumentBuffer<'q>,
            ) -> Result<IsNull, BoxDynError> {
                <&T as Encode<DB>>::encode(self, buf)
            }

            #[inline]
            fn produces(&self) -> Option<DB::TypeInfo> {
                (**self).produces()
            }

            #[inline]
            fn size_hint(&self) -> usize {
                (**self).size_hint()
            }
        }
    };
}

impl_encode_for_smartpointer!(Arc<T>);
impl_encode_for_smartpointer!(Box<T>);
impl_encode_for_smartpointer!(Rc<T>);

impl<'q, T, DB: Database> Encode<'q, DB> for Cow<'_, T>
where
    T: Encode<'q, DB>,
    T: ToOwned,
{
    #[inline]
    fn encode(self, buf: &mut <DB as Database>::ArgumentBuffer<'q>) -> Result<IsNull, BoxDynError> {
        <&T as Encode<DB>>::encode_by_ref(&self.as_ref(), buf)
    }

    #[inline]
    fn encode_by_ref(
        &self,
        buf: &mut <DB as Database>::ArgumentBuffer<'q>,
    ) -> Result<IsNull, BoxDynError> {
        <&T as Encode<DB>>::encode_by_ref(&self.as_ref(), buf)
    }

    #[inline]
    fn produces(&self) -> Option<DB::TypeInfo> {
        <&T as Encode<DB>>::produces(&self.as_ref())
    }

    #[inline]
    fn size_hint(&self) -> usize {
        <&T as Encode<DB>>::size_hint(&self.as_ref())
    }
}

Review comment (suggestion):
This might get weird with non-`'static` lifetimes. I would test that it's actually possible to bind with borrowed data.

Code after:
                &self,
                buf: &mut <DB as Database>::ArgumentBuffer<'q>,
            ) -> Result<IsNull, BoxDynError> {
                <&T as Encode<DB>>::encode(self, buf)
            }

            #[inline]
            fn produces(&self) -> Option<DB::TypeInfo> {
                (**self).produces()
            }

            #[inline]
            fn size_hint(&self) -> usize {
                (**self).size_hint()
            }
        }
    };
}

impl_encode_for_smartpointer!(Arc<T>);
impl_encode_for_smartpointer!(Box<T>);
impl_encode_for_smartpointer!(Rc<T>);

impl<'q, T, DB: Database> Encode<'q, DB> for Cow<'q, T>
where
    T: Encode<'q, DB>,
    T: ToOwned,
{
    #[inline]
    fn encode(self, buf: &mut <DB as Database>::ArgumentBuffer<'q>) -> Result<IsNull, BoxDynError> {
        <&T as Encode<DB>>::encode_by_ref(&self.as_ref(), buf)
    }

    #[inline]
    fn encode_by_ref(
        &self,
        buf: &mut <DB as Database>::ArgumentBuffer<'q>,
    ) -> Result<IsNull, BoxDynError> {
        <&T as Encode<DB>>::encode_by_ref(&self.as_ref(), buf)
    }

    #[inline]
    fn produces(&self) -> Option<DB::TypeInfo> {
        <&T as Encode<DB>>::produces(&self.as_ref())
    }

    #[inline]
    fn size_hint(&self) -> usize {
        <&T as Encode<DB>>::size_hint(&self.as_ref())
    }
}
Repository: launchbadge/sqlx
File: sqlx-core/src/types/mod.rs (Rust)

Code before:
    ($smart_pointer:ty) => {
        impl<T, DB: Database> Type<DB> for $smart_pointer
        where
            T: Type<DB>,
            T: ?Sized,
        {
            fn type_info() -> DB::TypeInfo {
                <T as Type<DB>>::type_info()
            }

            fn compatible(ty: &DB::TypeInfo) -> bool {
                <T as Type<DB>>::compatible(ty)
            }
        }
    };
}

impl_type_for_smartpointer!(Arc<T>);
impl_type_for_smartpointer!(Box<T>);
impl_type_for_smartpointer!(Rc<T>);

impl<T, DB: Database> Type<DB> for Cow<'_, T>
where
    T: Type<DB>,
    T: ToOwned,
    T: ?Sized,
{
    fn type_info() -> DB::TypeInfo {
        <T as Type<DB>>::type_info()
    }

    fn compatible(ty: &DB::TypeInfo) -> bool {
        <T as Type<DB>>::compatible(ty)
    }
}


Review comment (suggestion):
Weird way to style this. The requirements for a given type parameter should be grouped together.

```suggestion
    T: Type<DB> + ToOwned + ?Sized,
```

Code after:

    fn compatible(ty: &DB::TypeInfo) -> bool {
        ty.is_null() || <T as Type<DB>>::compatible(ty)
    }
}

macro_rules! impl_type_for_smartpointer {
    ($smart_pointer:ty) => {
        impl<T, DB: Database> Type<DB> for $smart_pointer
        where
            T: Type<DB> + ?Sized,
        {
            fn type_info() -> DB::TypeInfo {
                <T as Type<DB>>::type_info()
            }

            fn compatible(ty: &DB::TypeInfo) -> bool {
                <T as Type<DB>>::compatible(ty)
            }
        }
    };
}

impl_type_for_smartpointer!(Arc<T>);
impl_type_for_smartpointer!(Box<T>);
impl_type_for_smartpointer!(Rc<T>);

impl<T, DB: Database> Type<DB> for Cow<'_, T>
where
    // `ToOwned` is required here to satisfy `Cow`
    T: Type<DB> + ToOwned + ?Sized,
{
    fn type_info() -> DB::TypeInfo {
        <T as Type<DB>>::type_info()
    }

    fn compatible(ty: &DB::TypeInfo) -> bool {
        <T as Type<DB>>::compatible(ty)
    }
}
Repository: launchbadge/sqlx
File: sqlx-core/src/types/mod.rs (Rust)

Code before:
    fn type_info() -> DB::TypeInfo {
        <T as Type<DB>>::type_info()
    }

    fn compatible(ty: &DB::TypeInfo) -> bool {
        <T as Type<DB>>::compatible(ty)
    }
}

// for optionals, the underlying SQL type is identical
impl<T: Type<DB>, DB: Database> Type<DB> for Option<T> {
    fn type_info() -> DB::TypeInfo {
        <T as Type<DB>>::type_info()
    }

    fn compatible(ty: &DB::TypeInfo) -> bool {
        ty.is_null() || <T as Type<DB>>::compatible(ty)
    }
}

macro_rules! impl_type_for_smartpointer {
    ($smart_pointer:ty) => {
        impl<T, DB: Database> Type<DB> for $smart_pointer
        where
            T: Type<DB>,
            T: ?Sized,
        {
            fn type_info() -> DB::TypeInfo {
                <T as Type<DB>>::type_info()
            }

            fn compatible(ty: &DB::TypeInfo) -> bool {
                <T as Type<DB>>::compatible(ty)
            }
        }
    };
}

impl_type_for_smartpointer!(Arc<T>);
impl_type_for_smartpointer!(Box<T>);
impl_type_for_smartpointer!(Rc<T>);

impl<T, DB: Database> Type<DB> for Cow<'_, T>
where
    T: Type<DB>,
    T: ToOwned,
    T: ?Sized,
{
    fn type_info() -> DB::TypeInfo {
        <T as Type<DB>>::type_info()
    }

Review comment (suggestion):
```suggestion
            T: Type<DB> + ?Sized,
```

Code after:
    fn compatible(ty: &DB::TypeInfo) -> bool {
        Self::type_info().type_compatible(ty)
    }
}

// for references, the underlying SQL type is identical
impl<T: ?Sized + Type<DB>, DB: Database> Type<DB> for &'_ T {
    fn type_info() -> DB::TypeInfo {
        <T as Type<DB>>::type_info()
    }

    fn compatible(ty: &DB::TypeInfo) -> bool {
        <T as Type<DB>>::compatible(ty)
    }
}

// for optionals, the underlying SQL type is identical
impl<T: Type<DB>, DB: Database> Type<DB> for Option<T> {
    fn type_info() -> DB::TypeInfo {
        <T as Type<DB>>::type_info()
    }

    fn compatible(ty: &DB::TypeInfo) -> bool {
        ty.is_null() || <T as Type<DB>>::compatible(ty)
    }
}

macro_rules! impl_type_for_smartpointer {
    ($smart_pointer:ty) => {
        impl<T, DB: Database> Type<DB> for $smart_pointer
        where
            T: Type<DB> + ?Sized,
        {
            fn type_info() -> DB::TypeInfo {
                <T as Type<DB>>::type_info()
            }

            fn compatible(ty: &DB::TypeInfo) -> bool {
                <T as Type<DB>>::compatible(ty)
            }
        }
    };
}

impl_type_for_smartpointer!(Arc<T>);
impl_type_for_smartpointer!(Box<T>);
impl_type_for_smartpointer!(Rc<T>);

impl<T, DB: Database> Type<DB> for Cow<'_, T>
where
    // `ToOwned` is required here to satisfy `Cow`