ronantakizawa/codereview-bench huggingface dataset:
Repository: actix/actix-web
File: awc/src/middleware/redirect.rs (Rust)

Code before:
                    HttpResponse::InternalServerError().finish()
                }
            }

            App::new()
                .service(web::resource("/").route(web::to(root)))
                .service(web::resource("/test").route(web::to(test)))
        });

        let client = ClientBuilder::new()
            .header("custom", "value")
            .disable_redirects()
            .finish();
        let res = client.get(srv.url("/")).send().await.unwrap();
        assert_eq!(res.status().as_u16(), 302);

        let client = ClientBuilder::new().header("custom", "value").finish();
        let res = client.get(srv.url("/")).send().await.unwrap();
        assert_eq!(res.status().as_u16(), 200);

        let client = ClientBuilder::new().finish();
        let res = client
            .get(srv.url("/"))
            .insert_header(("custom", "value"))
            .send()
            .await
            .unwrap();
        assert_eq!(res.status().as_u16(), 200);
    }
}


Review comment (suggestion):
```suggestion
                    .get("custom")
                    == Some("value")
                {
```

Code after:
                    HttpResponse::InternalServerError().finish()
                }
            }

            App::new()
                .service(web::resource("/").route(web::to(root)))
                .service(web::resource("/test").route(web::to(test)))
        });

        let res = srv.post("/").send_body("Hello").await.unwrap();
        assert_eq!(res.status().as_u16(), 200);

        let res = srv.post("/").send().await.unwrap();
        assert_eq!(res.status().as_u16(), 200);
    }

    #[actix_rt::test]
    async fn test_redirect_headers() {
        let srv = actix_test::start(|| {
            async fn root(req: HttpRequest) -> HttpResponse {
                if req
                    .headers()
                    .get("custom")
                    .unwrap_or(&HeaderValue::from_str("").unwrap())
                    == "value"
                {
                    HttpResponse::Found()
                        .append_header(("location", "/test"))
                        .finish()
                } else {
                    HttpResponse::InternalServerError().finish()
                }
            }

            async fn test(req: HttpRequest) -> HttpResponse {
                if req
                    .headers()
                    .get("custom")
                    .unwrap_or(&HeaderValue::from_str("").unwrap())
                    == "value"
                {
                    HttpResponse::Ok().finish()
                } else {
                    HttpResponse::InternalServerError().finish()
                }
            }

            App::new()
                .service(web::resource("/").route(web::to(root)))
                .service(web::resource("/test").route(web::to(test)))
        });
Repository: actix/actix-web
File: actix-http/src/body/body.rs (Rust)

Code before:
    ///
    /// If your bytes container is owned, it may be cheaper to use a `From` impl.
    pub fn copy_from_slice(s: &[u8]) -> Self {
        Self::Bytes(Bytes::copy_from_slice(s))
    }

    #[doc(hidden)]
    #[deprecated(since = "4.0.0", note = "Renamed to `copy_from_slice`.")]
    pub fn from_slice(s: &[u8]) -> Self {
        Self::Bytes(Bytes::copy_from_slice(s))
    }
}

impl<B> AnyBody<B>
where
    B: MessageBody + 'static,
    B::Error: Into<Box<dyn StdError + 'static>>,
{
    /// Create body from generic message body.
    pub fn new(body: B) -> Self {
        Self::Body(body)
    }

    pub fn into_boxed(self) -> AnyBody {
        match self {
            AnyBody::None => AnyBody::new_boxed(()),
            AnyBody::Bytes(body) => AnyBody::new_boxed(body),
            AnyBody::Body(body) => AnyBody::new_boxed(body),
        }
    }
}

impl<B> MessageBody for AnyBody<B>
where
    B: MessageBody + Unpin,
    B::Error: Into<Box<dyn StdError>> + 'static,
{
    type Error = Error;

    fn size(&self) -> BodySize {
        match self {
            AnyBody::None => BodySize::None,
            AnyBody::Bytes(ref bin) => BodySize::Sized(bin.len() as u64),
            AnyBody::Body(ref body) => body.size(),
        }
    }

    fn poll_next(
        self: Pin<&mut Self>,
        cx: &mut Context<'_>,
    ) -> Poll<Option<Result<Bytes, Self::Error>>> {

Review comment (suggestion):
```rust
match self {
    Self::None => AnyBody::None,
    Self::Bytes(bytes) => AnyBody::Bytes(bytes)
}
```
This would keep all response with fixed in memory body unboxed.

Code after:

    /// Constructs new `AnyBody` instance from a slice of bytes by copying it.
    ///
    /// If your bytes container is owned, it may be cheaper to use a `From` impl.
    pub fn copy_from_slice(s: &[u8]) -> Self {
        Self::Bytes(Bytes::copy_from_slice(s))
    }

    #[doc(hidden)]
    #[deprecated(since = "4.0.0", note = "Renamed to `copy_from_slice`.")]
    pub fn from_slice(s: &[u8]) -> Self {
        Self::Bytes(Bytes::copy_from_slice(s))
    }
}

impl<B> AnyBody<B>
where
    B: MessageBody + 'static,
    B::Error: Into<Box<dyn StdError + 'static>>,
{
    /// Create body from generic message body.
    pub fn new(body: B) -> Self {
        Self::Body(body)
    }

    pub fn into_boxed(self) -> AnyBody {
        match self {
            Self::None => AnyBody::None,
            Self::Bytes(bytes) => AnyBody::Bytes(bytes),
            Self::Body(body) => AnyBody::new_boxed(body),
        }
    }
}

impl<B> MessageBody for AnyBody<B>
where
    B: MessageBody,
    B::Error: Into<Box<dyn StdError>> + 'static,
{
    type Error = Error;

    fn size(&self) -> BodySize {
        match self {
            AnyBody::None => BodySize::None,
            AnyBody::Bytes(ref bin) => BodySize::Sized(bin.len() as u64),
            AnyBody::Body(ref body) => body.size(),
        }
    }

    fn poll_next(
        self: Pin<&mut Self>,
Repository: actix/actix-web
File: actix-http/src/body/body.rs (Rust)

Code before:
use std::{
    borrow::Cow,
    error::Error as StdError,
    fmt, mem,
    pin::Pin,
    task::{Context, Poll},
};

use bytes::{Bytes, BytesMut};
use futures_core::Stream;

use crate::error::Error;

use super::{BodySize, BodyStream, MessageBody, MessageBodyMapErr, SizedStream};

pub type Body = AnyBody;

/// Represents various types of HTTP message body.
#[derive(Clone)]
pub enum AnyBody<B = BoxBody> {
    /// Empty response. `Content-Length` header is not set.
    None,

    /// Specific response body.
    Bytes(Bytes),

    /// Generic / Other message body.
    Body(B),
}

impl AnyBody {
    /// Constructs a new, empty body.
    pub fn empty() -> Self {
        Self::Bytes(Bytes::new())
    }

    /// Create boxed body from generic message body.
    pub fn new_boxed<B>(body: B) -> Self
    where
        B: MessageBody + 'static,
        B::Error: Into<Box<dyn StdError + 'static>>,
    {
        Self::Body(BoxBody::from_body(body))
    }

    /// Constructs new `AnyBody` instance from a slice of bytes by copying it.
    ///
    /// If your bytes container is owned, it may be cheaper to use a `From` impl.
    pub fn copy_from_slice(s: &[u8]) -> Self {

Review comment (suggestion):
`AnyBody::Bytes` may better be worded as in memory response body.
Typically the generic body variant is for async streaming where the vaule is not able and/or desirable to be in memory as a whole. 
This would help consumer to construct the correct variant.

Code after:
use std::{
    borrow::Cow,
    error::Error as StdError,
    fmt, mem,
    pin::Pin,
    task::{Context, Poll},
};

use bytes::{Bytes, BytesMut};
use futures_core::Stream;
use pin_project::pin_project;

use crate::error::Error;

use super::{BodySize, BodyStream, MessageBody, MessageBodyMapErr, SizedStream};

pub type Body = AnyBody;

/// Represents various types of HTTP message body.
#[pin_project(project = AnyBodyProj)]
#[derive(Clone)]
pub enum AnyBody<B = BoxBody> {
    /// Empty response. `Content-Length` header is not set.
    None,

    /// Complete, in-memory response body.
    Bytes(Bytes),

    /// Generic / Other message body.
    Body(#[pin] B),
}

impl AnyBody {
    /// Constructs a new, empty body.
    pub fn empty() -> Self {
        Self::Bytes(Bytes::new())
    }

    /// Create boxed body from generic message body.
    pub fn new_boxed<B>(body: B) -> Self
    where
        B: MessageBody + 'static,
        B::Error: Into<Box<dyn StdError + 'static>>,
    {
        Self::Body(BoxBody::from_body(body))
    }

    /// Constructs new `AnyBody` instance from a slice of bytes by copying it.
    ///
Repository: actix/actix-web
File: actix-http/src/body/none.rs (Rust)

Code before:
use std::{
    convert::Infallible,
    pin::Pin,
    task::{Context, Poll},
};

use bytes::Bytes;

use super::{BodySize, MessageBody};

/// Body type for responses that forbid payloads.
///
/// Distinct from an empty response which would contain a Content-Length header.
///
/// For an "empty" body, use `()` or `Bytes::new()`.
#[derive(Debug, Clone, Copy, Default)]
#[non_exhaustive]
pub struct None;

impl None {
    /// Constructs new "none" body.
    #[inline]
    pub fn new() -> Self {
        None
    }
}

impl MessageBody for None {
    type Error = Infallible;

    #[inline]
    fn size(&self) -> BodySize {
        BodySize::None
    }

    #[inline]
    fn poll_next(
        self: Pin<&mut Self>,
        _cx: &mut Context<'_>,
    ) -> Poll<Option<Result<Bytes, Self::Error>>> {
        Poll::Ready(Option::None)
    }
}


Review comment (suggestion):
I think I understand why you removed this; the use case for these methods also should go along with checking the size hint for `None` and bailing and, as such, these methods will/should never be called. So for me there are two options:
- Keep them and allow for some unexpected, exotic use case where they _are_ reasonably called.
- Use a custom panic message that indicates that size hint should be checked first.

Code after:

impl MessageBody for None {
    type Error = Infallible;

    #[inline]
    fn size(&self) -> BodySize {
        BodySize::None
    }

    #[inline]
    fn poll_next(
        self: Pin<&mut Self>,
        _cx: &mut Context<'_>,
    ) -> Poll<Option<Result<Bytes, Self::Error>>> {
        Poll::Ready(Option::None)
    }

    #[inline]
    fn is_complete_body(&self) -> bool {
        true
    }

    #[inline]
    fn take_complete_body(&mut self) -> Bytes {
        Bytes::new()
    }
}
Repository: actix/actix-web
File: actix-http/src/body/boxed.rs (Rust)

Code before:
use std::{
    error::Error as StdError,
    fmt,
    pin::Pin,
    task::{Context, Poll},
};

use bytes::Bytes;

use super::{BodySize, MessageBody, MessageBodyMapErr};

/// A boxed message body with boxed errors.
pub struct BoxBody(BoxBodyInner);

enum BoxBodyInner {
    None,
    Bytes(Bytes),
    Stream(Pin<Box<dyn MessageBody<Error = Box<dyn StdError>>>>),
}

impl BoxBody {
    /// Same as `MessageBody::boxed`.
    ///
    /// If the body type to wrap is unknown or generic it is better to use [`MessageBody::boxed`] to
    /// avoid double boxing.
    #[inline]
    pub fn new<B>(body: B) -> Self
    where
        B: MessageBody + 'static,
    {
        match body.size() {
            BodySize::None => Self(BoxBodyInner::None),
            _ => match body.try_into_bytes() {
                Ok(bytes) => Self(BoxBodyInner::Bytes(bytes)),
                Err(body) => {
                    let body = MessageBodyMapErr::new(body, Into::into);
                    Self(BoxBodyInner::Stream(Box::pin(body)))
                }
            },
        }
    }

Review comment (suggestion):
i kinda think this should wrap `body::None` and delegate so that the behavior is guaranteed to be the same

Code after:
use std::{
    error::Error as StdError,
    fmt,
    pin::Pin,
    task::{Context, Poll},
};

use bytes::Bytes;

use super::{BodySize, MessageBody, MessageBodyMapErr};
use crate::body;

/// A boxed message body with boxed errors.
pub struct BoxBody(BoxBodyInner);

enum BoxBodyInner {
    None(body::None),
    Bytes(Bytes),
    Stream(Pin<Box<dyn MessageBody<Error = Box<dyn StdError>>>>),
}

impl BoxBody {
    /// Same as `MessageBody::boxed`.
    ///
    /// If the body type to wrap is unknown or generic it is better to use [`MessageBody::boxed`] to
    /// avoid double boxing.
    #[inline]
    pub fn new<B>(body: B) -> Self
    where
        B: MessageBody + 'static,
    {
        match body.size() {
            BodySize::None => Self(BoxBodyInner::None(body::None)),
            _ => match body.try_into_bytes() {
                Ok(bytes) => Self(BoxBodyInner::Bytes(bytes)),
                Err(body) => {
                    let body = MessageBodyMapErr::new(body, Into::into);
                    Self(BoxBodyInner::Stream(Box::pin(body)))
                }
            },
        }
Repository: actix/actix-web
File: src/config.rs (Rust)

Code before:
    addr: SocketAddr,
}

impl AppConfig {
    pub(crate) fn new(secure: bool, host: String, addr: SocketAddr) -> Self {
        AppConfig { secure, host, addr }
    }

    /// Needed in actix-test crate. Semver exempt.
    #[doc(hidden)]
    pub fn __priv_test_new(secure: bool, host: String, addr: SocketAddr) -> Self {
        AppConfig::new(secure, host, addr)
    }

    /// Server host name.
    ///
    /// Host name is used by application router as a hostname for URL generation.
    /// Check [ConnectionInfo](super::dev::ConnectionInfo::host())
    /// documentation for more information.
    ///
    /// By default host name is set to a "localhost" value.
    pub fn host(&self) -> &str {
        &self.host
    }

    /// Returns true if connection is secure (ie. running over `https:`).
    pub fn secure(&self) -> bool {
        self.secure
    }

    /// Returns the socket address of the local half of this TCP connection
    pub fn local_addr(&self) -> SocketAddr {
        self.addr
    }

    #[cfg(test)]
    pub(crate) fn set_host(&mut self, host: &str) {
        self.host = host.to_owned();
    }
}

impl Default for AppConfig {
    fn default() -> Self {
        AppConfig::new(
            false,
            "localhost:8080".to_owned(),
            "127.0.0.1:8080".parse().unwrap(),
        )
    }
}


Review comment (suggestion):
```suggestion
    /// Returns true if connection is secure (i.e. running over `https:`).
```

Code after:
    addr: SocketAddr,
}

impl AppConfig {
    pub(crate) fn new(secure: bool, host: String, addr: SocketAddr) -> Self {
        AppConfig { secure, host, addr }
    }

    /// Needed in actix-test crate. Semver exempt.
    #[doc(hidden)]
    pub fn __priv_test_new(secure: bool, host: String, addr: SocketAddr) -> Self {
        AppConfig::new(secure, host, addr)
    }

    /// Server host name.
    ///
    /// Host name is used by application router as a hostname for URL generation.
    /// Check [ConnectionInfo](super::dev::ConnectionInfo::host())
    /// documentation for more information.
    ///
    /// By default host name is set to a "localhost" value.
    pub fn host(&self) -> &str {
        &self.host
    }

    /// Returns true if connection is secure (i.e., running over `https:`).
    pub fn secure(&self) -> bool {
        self.secure
    }

    /// Returns the socket address of the local half of this TCP connection
    pub fn local_addr(&self) -> SocketAddr {
        self.addr
    }

    #[cfg(test)]
    pub(crate) fn set_host(&mut self, host: &str) {
        self.host = host.to_owned();
    }
}

impl Default for AppConfig {
    fn default() -> Self {
        AppConfig::new(
            false,
            "localhost:8080".to_owned(),
            "127.0.0.1:8080".parse().unwrap(),
        )
    }
}
Repository: actix/actix-web
File: actix-web-codegen/src/lib.rs (Rust)

Code before:
/// # Examples
/// ```
/// # use actix_web::HttpResponse;
/// # use actix_web_codegen::route;
/// #[route("/test", method = "GET", method = "HEAD")]
/// async fn example() -> HttpResponse {
///     HttpResponse::Ok().finish()
/// }
/// ```
#[proc_macro_attribute]
pub fn route(args: TokenStream, input: TokenStream) -> TokenStream {
    route::with_method(None, args, input)
}

/// Creates resource handler, allowing multiple HTTP methods and paths.
///
/// # Syntax
/// ```plain
/// #[routes]
/// #[<method>("path", ...)]
/// #[<method>("path", ...)]
/// ...
/// ```
///
/// # Attributes
/// The `routes` macro it self has no parameters, but allows specifying the attribute macros for
/// the different methods, e.g. [`GET`](macro@get) or [`POST`](macro@post).
///
/// These helper attributes take the same parameters as the [single method handlers](crate#single-method-handler).
///
/// # Examples
/// ```
/// # use actix_web::HttpResponse;
/// # use actix_web_codegen::routes;
/// #[routes]
/// #[get("/test")]
/// #[get("/test2")]
/// #[delete("/test")]
/// async fn example() -> HttpResponse {
///     HttpResponse::Ok().finish()
/// }
/// ```
#[proc_macro_attribute]
pub fn routes(_: TokenStream, input: TokenStream) -> TokenStream {
    route::with_methods(input)
}

macro_rules! method_macro {
    ($variant:ident, $method:ident) => {
#[doc = concat!("Creates route handler with `actix_web::guard::", stringify!($variant), "`.")]
///

Review comment (suggestion):
```suggestion
/// The `routes` macro itself has no parameters, but allows specifying the attribute macros for
```

Code after:
/// # Examples
/// ```
/// # use actix_web::HttpResponse;
/// # use actix_web_codegen::route;
/// #[route("/test", method = "GET", method = "HEAD")]
/// async fn example() -> HttpResponse {
///     HttpResponse::Ok().finish()
/// }
/// ```
#[proc_macro_attribute]
pub fn route(args: TokenStream, input: TokenStream) -> TokenStream {
    route::with_method(None, args, input)
}

/// Creates resource handler, allowing multiple HTTP methods and paths.
///
/// # Syntax
/// ```plain
/// #[routes]
/// #[<method>("path", ...)]
/// #[<method>("path", ...)]
/// ...
/// ```
///
/// # Attributes
/// The `routes` macro itself has no parameters, but allows specifying the attribute macros for
/// the multiple paths and/or methods, e.g. [`GET`](macro@get) and [`POST`](macro@post).
///
/// These helper attributes take the same parameters as the [single method handlers](crate#single-method-handler).
///
/// # Examples
/// ```
/// # use actix_web::HttpResponse;
/// # use actix_web_codegen::routes;
/// #[routes]
/// #[get("/test")]
/// #[get("/test2")]
/// #[delete("/test")]
/// async fn example() -> HttpResponse {
///     HttpResponse::Ok().finish()
/// }
/// ```
#[proc_macro_attribute]
pub fn routes(_: TokenStream, input: TokenStream) -> TokenStream {
    route::with_methods(input)
}

macro_rules! method_macro {
    ($variant:ident, $method:ident) => {
#[doc = concat!("Creates route handler with `actix_web::guard::", stringify!($variant), "`.")]
///
Repository: actix/actix-web
File: actix-web-codegen/src/lib.rs (Rust)

Code before:
/// ```
/// # use actix_web::HttpResponse;
/// # use actix_web_codegen::route;
/// #[route("/test", method = "GET", method = "HEAD")]
/// async fn example() -> HttpResponse {
///     HttpResponse::Ok().finish()
/// }
/// ```
#[proc_macro_attribute]
pub fn route(args: TokenStream, input: TokenStream) -> TokenStream {
    route::with_method(None, args, input)
}

/// Creates resource handler, allowing multiple HTTP methods and paths.
///
/// # Syntax
/// ```plain
/// #[routes]
/// #[<method>("path", ...)]
/// #[<method>("path", ...)]
/// ...
/// ```
///
/// # Attributes
/// The `routes` macro it self has no parameters, but allows specifying the attribute macros for
/// the different methods, e.g. [`GET`](macro@get) or [`POST`](macro@post).
///
/// These helper attributes take the same parameters as the [single method handlers](crate#single-method-handler).
///
/// # Examples
/// ```
/// # use actix_web::HttpResponse;
/// # use actix_web_codegen::routes;
/// #[routes]
/// #[get("/test")]
/// #[get("/test2")]
/// #[delete("/test")]
/// async fn example() -> HttpResponse {
///     HttpResponse::Ok().finish()
/// }
/// ```
#[proc_macro_attribute]
pub fn routes(_: TokenStream, input: TokenStream) -> TokenStream {
    route::with_methods(input)
}

macro_rules! method_macro {
    ($variant:ident, $method:ident) => {
#[doc = concat!("Creates route handler with `actix_web::guard::", stringify!($variant), "`.")]
///
/// # Syntax

Review comment (suggestion):
```suggestion
/// the multiple paths and/or methods, e.g. [`GET`](macro@get) and [`POST`](macro@post).
```

Code after:
/// ```
/// # use actix_web::HttpResponse;
/// # use actix_web_codegen::route;
/// #[route("/test", method = "GET", method = "HEAD")]
/// async fn example() -> HttpResponse {
///     HttpResponse::Ok().finish()
/// }
/// ```
#[proc_macro_attribute]
pub fn route(args: TokenStream, input: TokenStream) -> TokenStream {
    route::with_method(None, args, input)
}

/// Creates resource handler, allowing multiple HTTP methods and paths.
///
/// # Syntax
/// ```plain
/// #[routes]
/// #[<method>("path", ...)]
/// #[<method>("path", ...)]
/// ...
/// ```
///
/// # Attributes
/// The `routes` macro itself has no parameters, but allows specifying the attribute macros for
/// the multiple paths and/or methods, e.g. [`GET`](macro@get) and [`POST`](macro@post).
///
/// These helper attributes take the same parameters as the [single method handlers](crate#single-method-handler).
///
/// # Examples
/// ```
/// # use actix_web::HttpResponse;
/// # use actix_web_codegen::routes;
/// #[routes]
/// #[get("/test")]
/// #[get("/test2")]
/// #[delete("/test")]
/// async fn example() -> HttpResponse {
///     HttpResponse::Ok().finish()
/// }
/// ```
#[proc_macro_attribute]
pub fn routes(_: TokenStream, input: TokenStream) -> TokenStream {
    route::with_methods(input)
}

macro_rules! method_macro {
    ($variant:ident, $method:ident) => {
#[doc = concat!("Creates route handler with `actix_web::guard::", stringify!($variant), "`.")]
///
/// # Syntax