ronantakizawa/codereview-bench huggingface dataset:
Repository: actix/actix-web
File: src/http/header/content_language.rs (Rust)

Code before:
    /// ```
    ///
    /// # Example values
    ///
    /// * `da`
    /// * `mi, en`
    ///
    /// # Examples
    ///
    /// ```
    /// use language_tags::LanguageTag;
    /// use actix_web::HttpResponse;
    /// use actix_web::http::header::{ContentLanguage, qitem};
    ///
    /// let mut builder = HttpResponse::Ok();
    /// builder.insert_header(
    ///     ContentLanguage(vec![
    ///         qitem(LanguageTag::parse("en").unwrap()),
    ///     ])
    /// );
    /// ```
    ///
    /// ```
    /// use language_tags::LanguageTag;
    /// use actix_web::HttpResponse;
    /// use actix_web::http::header::{ContentLanguage, qitem};
    ///
    /// let mut builder = HttpResponse::Ok();
    /// builder.insert_header(
    ///     ContentLanguage(vec![
    ///         qitem(LanguageTag::parse("da").unwrap()),
    ///         qitem(LanguageTag::parse("en-GB").unwrap()),
    ///     ])
    /// );
    /// ```
    (ContentLanguage, CONTENT_LANGUAGE) => (QualityItem<LanguageTag>)+

    test_content_language {
        crate::__common_header_test!(test1, vec![b"da"]);
        crate::__common_header_test!(test2, vec![b"mi, en"]);
    }
}


Review comment (suggestion):
```suggestion
    /// use actix_web::http::header::{ContentLanguage, LanguageTag, qitem};
```

Code after:
    /// ```
    ///
    /// # Example values
    ///
    /// * `da`
    /// * `mi, en`
    ///
    /// # Examples
    ///
    /// ```
    /// use actix_web::HttpResponse;
    /// use actix_web::http::header::{ContentLanguage, LanguageTag, qitem};
    ///
    /// let mut builder = HttpResponse::Ok();
    /// builder.insert_header(
    ///     ContentLanguage(vec![
    ///         qitem(LanguageTag::parse("en").unwrap()),
    ///     ])
    /// );
    /// ```
    ///
    /// ```
    /// use actix_web::HttpResponse;
    /// use actix_web::http::header::{ContentLanguage, LanguageTag, qitem};
    ///
    /// let mut builder = HttpResponse::Ok();
    /// builder.insert_header(
    ///     ContentLanguage(vec![
    ///         qitem(LanguageTag::parse("da").unwrap()),
    ///         qitem(LanguageTag::parse("en-GB").unwrap()),
    ///     ])
    /// );
    /// ```
    (ContentLanguage, CONTENT_LANGUAGE) => (QualityItem<LanguageTag>)+

    test_content_language {
        crate::__common_header_test!(test1, vec![b"da"]);
        crate::__common_header_test!(test2, vec![b"mi, en"]);
    }
}
Repository: actix/actix-web
File: src/extract.rs (Rust)

Code before:
//! Request extractors

use std::{
    future::Future,
    pin::Pin,
    task::{Context, Poll},
};

use actix_utils::future::{ready, Ready};
use futures_core::ready;

use crate::{dev::Payload, Error, HttpRequest};

/// [`FromRequest`] is one of the most commonly used trait, although some may not realize it.
///
/// A type that implements [`FromRequest`] is called an **extractor** and can be extracted
/// from the request, for example [`Json`], [`Form`], [`Path`] and so on.
///
/// An extractor can be customized by injecting the corresponding configuration with:
///
/// - [`App::data()`](`crate::App::data`)
/// - [`App::app_data()`](`crate::App::app_data`)
/// - [`Scope::data()`](`crate::Scope::data`)
/// - [`Scope::app_data()`](`crate::Scope::app_data`)
/// - [`Resource::data()`](`crate::Resource::data`)
/// - [`Resource::app_data()`](`crate::Resource::app_data`)
///
/// Here are some built-in extractors and their corresponding configuration.
/// Please refer to the respective documents for details.
///
/// | Extractor   | Configuration     |
/// |-------------|-------------------|
/// | [`Json`]    | [`JsonConfig`]    |
/// | [`Form`]    | [`FormConfig`]    |
/// | [`Path`]    | [`PathConfig`]    |
/// | [`Query`]   | [`QueryConfig`]   |
/// | [`Payload`] | [`PayloadConfig`] |
/// | [`String`]  | [`PayloadConfig`] |
/// | [`Bytes`]   | [`PayloadConfig`] |
///
/// [`Json`]: crate::web::Json

Review comment (refactor):
```suggestion
/// A type that implements [`FromRequest`] is called an **extractor** and can extract data
```

Code after:
//! Request extractors

use std::{
    convert::Infallible,
    future::Future,
    pin::Pin,
    task::{Context, Poll},
};

use actix_http::http::{Method, Uri};
use actix_utils::future::{ok, Ready};
use futures_core::ready;

use crate::{dev::Payload, Error, HttpRequest};

/// A type that implements [`FromRequest`] is called an **extractor** and can extract data
/// from the request. Examples of types that implement this trait are [`Json`], [`Form`], [`Path`].
///
/// An extractor can be customized by injecting the corresponding configuration with one of:
///
/// - [`App::app_data()`](`crate::App::app_data`)
/// - [`Scope::app_data()`](`crate::Scope::app_data`)
/// - [`Resource::app_data()`](`crate::Resource::app_data`)
///
/// Here are some built-in extractors and their corresponding configuration.
/// Please refer to the respective documentation for details.
///
/// | Extractor   | Configuration     |
/// |-------------|-------------------|
/// | [`Json`]    | [`JsonConfig`]    |
/// | [`Form`]    | [`FormConfig`]    |
/// | [`Path`]    | [`PathConfig`]    |
/// | [`Query`]   | [`QueryConfig`]   |
/// | [`Payload`] | [`PayloadConfig`] |
/// | [`String`]  | [`PayloadConfig`] |
/// | [`Bytes`]   | [`PayloadConfig`] |
///
/// [`Json`]: crate::web::Json
/// [`JsonConfig`]: crate::web::JsonConfig
/// [`Form`]: crate::web::Form
/// [`FormConfig`]: crate::web::FormConfig
Repository: actix/actix-web
File: src/extract.rs (Rust)

Code before:
//! Request extractors

use std::{
    future::Future,
    pin::Pin,
    task::{Context, Poll},
};

use actix_utils::future::{ready, Ready};
use futures_core::ready;

use crate::{dev::Payload, Error, HttpRequest};

/// [`FromRequest`] is one of the most commonly used trait, although some may not realize it.
///
/// A type that implements [`FromRequest`] is called an **extractor** and can be extracted
/// from the request, for example [`Json`], [`Form`], [`Path`] and so on.
///
/// An extractor can be customized by injecting the corresponding configuration with:
///
/// - [`App::data()`](`crate::App::data`)
/// - [`App::app_data()`](`crate::App::app_data`)
/// - [`Scope::data()`](`crate::Scope::data`)
/// - [`Scope::app_data()`](`crate::Scope::app_data`)
/// - [`Resource::data()`](`crate::Resource::data`)
/// - [`Resource::app_data()`](`crate::Resource::app_data`)
///
/// Here are some built-in extractors and their corresponding configuration.
/// Please refer to the respective documents for details.
///
/// | Extractor   | Configuration     |
/// |-------------|-------------------|
/// | [`Json`]    | [`JsonConfig`]    |
/// | [`Form`]    | [`FormConfig`]    |
/// | [`Path`]    | [`PathConfig`]    |
/// | [`Query`]   | [`QueryConfig`]   |
/// | [`Payload`] | [`PayloadConfig`] |
/// | [`String`]  | [`PayloadConfig`] |
/// | [`Bytes`]   | [`PayloadConfig`] |
///
/// [`Json`]: crate::web::Json
/// [`JsonConfig`]: crate::web::JsonConfig

Review comment (suggestion):
```suggestion
/// from the request. Examples of types that implement this trait are [`Json`], [`Form`], [`Path`].
```

Code after:
//! Request extractors

use std::{
    convert::Infallible,
    future::Future,
    pin::Pin,
    task::{Context, Poll},
};

use actix_http::http::{Method, Uri};
use actix_utils::future::{ok, Ready};
use futures_core::ready;

use crate::{dev::Payload, Error, HttpRequest};

/// A type that implements [`FromRequest`] is called an **extractor** and can extract data
/// from the request. Examples of types that implement this trait are [`Json`], [`Form`], [`Path`].
///
/// An extractor can be customized by injecting the corresponding configuration with one of:
///
/// - [`App::app_data()`](`crate::App::app_data`)
/// - [`Scope::app_data()`](`crate::Scope::app_data`)
/// - [`Resource::app_data()`](`crate::Resource::app_data`)
///
/// Here are some built-in extractors and their corresponding configuration.
/// Please refer to the respective documentation for details.
///
/// | Extractor   | Configuration     |
/// |-------------|-------------------|
/// | [`Json`]    | [`JsonConfig`]    |
/// | [`Form`]    | [`FormConfig`]    |
/// | [`Path`]    | [`PathConfig`]    |
/// | [`Query`]   | [`QueryConfig`]   |
/// | [`Payload`] | [`PayloadConfig`] |
/// | [`String`]  | [`PayloadConfig`] |
/// | [`Bytes`]   | [`PayloadConfig`] |
///
/// [`Json`]: crate::web::Json
/// [`JsonConfig`]: crate::web::JsonConfig
/// [`Form`]: crate::web::Form
/// [`FormConfig`]: crate::web::FormConfig
/// [`Path`]: crate::web::Path
Repository: actix/actix-web
File: src/extract.rs (Rust)

Code before:
//! Request extractors

use std::{
    future::Future,
    pin::Pin,
    task::{Context, Poll},
};

use actix_utils::future::{ready, Ready};
use futures_core::ready;

use crate::{dev::Payload, Error, HttpRequest};

/// [`FromRequest`] is one of the most commonly used trait, although some may not realize it.
///
/// A type that implements [`FromRequest`] is called an **extractor** and can be extracted
/// from the request, for example [`Json`], [`Form`], [`Path`] and so on.
///
/// An extractor can be customized by injecting the corresponding configuration with:
///
/// - [`App::data()`](`crate::App::data`)
/// - [`App::app_data()`](`crate::App::app_data`)
/// - [`Scope::data()`](`crate::Scope::data`)
/// - [`Scope::app_data()`](`crate::Scope::app_data`)
/// - [`Resource::data()`](`crate::Resource::data`)
/// - [`Resource::app_data()`](`crate::Resource::app_data`)
///
/// Here are some built-in extractors and their corresponding configuration.
/// Please refer to the respective documents for details.
///
/// | Extractor   | Configuration     |
/// |-------------|-------------------|
/// | [`Json`]    | [`JsonConfig`]    |
/// | [`Form`]    | [`FormConfig`]    |
/// | [`Path`]    | [`PathConfig`]    |
/// | [`Query`]   | [`QueryConfig`]   |
/// | [`Payload`] | [`PayloadConfig`] |
/// | [`String`]  | [`PayloadConfig`] |
/// | [`Bytes`]   | [`PayloadConfig`] |

Review comment (suggestion):
IMHO this line is a bit bogus since it doesn't provide a user with any meaningful information. 
I'd suggest removing it completely.

Code after:
//! Request extractors

use std::{
    convert::Infallible,
    future::Future,
    pin::Pin,
    task::{Context, Poll},
};

use actix_http::http::{Method, Uri};
use actix_utils::future::{ok, Ready};
use futures_core::ready;

use crate::{dev::Payload, Error, HttpRequest};

/// A type that implements [`FromRequest`] is called an **extractor** and can extract data
/// from the request. Examples of types that implement this trait are [`Json`], [`Form`], [`Path`].
///
/// An extractor can be customized by injecting the corresponding configuration with one of:
///
/// - [`App::app_data()`](`crate::App::app_data`)
/// - [`Scope::app_data()`](`crate::Scope::app_data`)
/// - [`Resource::app_data()`](`crate::Resource::app_data`)
///
/// Here are some built-in extractors and their corresponding configuration.
/// Please refer to the respective documentation for details.
///
/// | Extractor   | Configuration     |
/// |-------------|-------------------|
/// | [`Json`]    | [`JsonConfig`]    |
/// | [`Form`]    | [`FormConfig`]    |
/// | [`Path`]    | [`PathConfig`]    |
/// | [`Query`]   | [`QueryConfig`]   |
/// | [`Payload`] | [`PayloadConfig`] |
/// | [`String`]  | [`PayloadConfig`] |
/// | [`Bytes`]   | [`PayloadConfig`] |
///
/// [`Json`]: crate::web::Json
/// [`JsonConfig`]: crate::web::JsonConfig
Repository: actix/actix-web
File: src/types/form.rs (Rust)

Code before:
///     .service(index);
/// ```
#[derive(Clone)]
pub struct FormConfig {
    limit: usize,
    err_handler: FormErrHandler,
}

impl FormConfig {
    /// Set maximum accepted payload size. By default this limit is 16kB.
    pub fn limit(mut self, limit: usize) -> Self {
        self.limit = limit;
        self
    }

    /// Set custom error handler
    pub fn error_handler<F>(mut self, f: F) -> Self
    where
        F: Fn(UrlencodedError, &HttpRequest) -> Error + 'static,
    {
        self.err_handler = Some(Rc::new(f));
        self
    }

    /// Extract payload config from app data. Check both `T` and `Data<T>`, in that order, and fall
    /// back to the default payload config.
    fn from_req(req: &HttpRequest) -> &Self {
        req.app_data::<Self>()
            .or_else(|| req.app_data::<web::Data<Self>>().map(|d| d.as_ref()))
            .unwrap_or(&DEFAULT_CONFIG)
    }
}

/// Allow shared refs used as default.
const DEFAULT_CONFIG: FormConfig = FormConfig {
    limit: 16_384, // 2^14 bytes (~16kB)
    err_handler: None,
};

impl Default for FormConfig {
    fn default() -> Self {
        DEFAULT_CONFIG.clone()
    }
}

/// Future that resolves to some `T` when parsed from a URL encoded payload.
///
/// Form can be deserialized from any type `T` that implements [`serde::Deserialize`].
///
/// Returns error if:
/// - content type is not `application/x-www-form-urlencoded`

Review comment (refactor):
```suggestion
    /// Extract payload config from app data.
    ///
    /// Checks both `T` and `Data<T>`, in that order, and falls back to the default payload config.
```

Code after:
///     Ok(format!("Welcome {}!", form.username))
/// }
///
/// App::new()
///     .app_data(web::FormConfig::default().limit(4096))
///     .service(index);
/// ```
#[derive(Clone)]
pub struct FormConfig {
    limit: usize,
    err_handler: FormErrHandler,
}

impl FormConfig {
    /// Set maximum accepted payload size. By default this limit is 16kB.
    pub fn limit(mut self, limit: usize) -> Self {
        self.limit = limit;
        self
    }

    /// Set custom error handler
    pub fn error_handler<F>(mut self, f: F) -> Self
    where
        F: Fn(UrlencodedError, &HttpRequest) -> Error + 'static,
    {
        self.err_handler = Some(Rc::new(f));
        self
    }

    /// Extract payload config from app data.
    ///
    /// Checks both `T` and `Data<T>`, in that order, and falls back to the default payload config.
    fn from_req(req: &HttpRequest) -> &Self {
        req.app_data::<Self>()
            .or_else(|| req.app_data::<web::Data<Self>>().map(|d| d.as_ref()))
            .unwrap_or(&DEFAULT_CONFIG)
    }
}

/// Allow shared refs used as default.
const DEFAULT_CONFIG: FormConfig = FormConfig {
    limit: 16_384, // 2^14 bytes (~16kB)
    err_handler: None,
};

impl Default for FormConfig {
    fn default() -> Self {
        DEFAULT_CONFIG
    }
}
Repository: actix/actix-web
File: actix-http/src/encoding/decoder.rs (Rust)

Code before:
//! Stream decoders.

use std::{
    future::Future,
    io::{self, Write as _},
    pin::Pin,
    task::{Context, Poll},
};

use actix_rt::task::{spawn_blocking, JoinHandle};
#[cfg(feature = "compress-br")]
use brotli2::write::BrotliDecoder;
use bytes::Bytes;
#[cfg(feature = "compress-gz")]
use flate2::write::{GzDecoder, ZlibDecoder};
use futures_core::{ready, Stream};
#[cfg(feature = "compress-zstd")]
use zstd::stream::write::Decoder as ZstdDecoder;

use crate::{
    encoding::Writer,
    error::{BlockingError, PayloadError},
    http::header::{ContentEncoding, HeaderMap, CONTENT_ENCODING},
};

const MAX_CHUNK_SIZE_DECODE_IN_PLACE: usize = 2049;

pub struct Decoder<S> {
    decoder: Option<ContentDecoder>,
    stream: S,
    eof: bool,
    fut: Option<JoinHandle<Result<(Option<Bytes>, ContentDecoder), io::Error>>>,
}

impl<S> Decoder<S>
where

Review comment (suggestion):
Totally nitpicky and a matter of taste, but I personally find import sequences combined with a lot of `cfg(...)` hard to read. I think it's better to separate `compress-*` imports with an empty line, so we have a more or less clean sequence of unconditional imports and then a sequence of imports guarded by `cfg(...)`.

Code after:
//! Stream decoders.

use std::{
    future::Future,
    io::{self, Write as _},
    pin::Pin,
    task::{Context, Poll},
};

use actix_rt::task::{spawn_blocking, JoinHandle};
use bytes::Bytes;
use futures_core::{ready, Stream};

#[cfg(feature = "compress-brotli")]
use brotli2::write::BrotliDecoder;

#[cfg(feature = "compress-gzip")]
use flate2::write::{GzDecoder, ZlibDecoder};

#[cfg(feature = "compress-zstd")]
use zstd::stream::write::Decoder as ZstdDecoder;

use crate::{
    encoding::Writer,
    error::{BlockingError, PayloadError},
    http::header::{ContentEncoding, HeaderMap, CONTENT_ENCODING},
};

const MAX_CHUNK_SIZE_DECODE_IN_PLACE: usize = 2049;

pub struct Decoder<S> {
    decoder: Option<ContentDecoder>,
    stream: S,
    eof: bool,
    fut: Option<JoinHandle<Result<(Option<Bytes>, ContentDecoder), io::Error>>>,
}
Repository: actix/actix-web
File: src/info.rs (Rust)

Code before:
    pub fn into_inner(self) -> SocketAddr {
        self.0
    }
}

#[derive(Debug, Display, Error)]
#[display(fmt = "Missing peer address")]
pub struct MissingPeerAddr;

impl ResponseError for MissingPeerAddr {}

impl FromRequest for PeerAddr {
    type Error = MissingPeerAddr;
    type Future = Ready<Result<Self, Self::Error>>;
    type Config = ();

    fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future {
        match req.peer_addr() {
            Some(addr) => ok(PeerAddr(addr)),
            None => {
                log::error!("Missing peer address.");
                err(MissingPeerAddr)
            }
        }
    }
}

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

    #[test]
    fn test_forwarded() {
        let req = TestRequest::default().to_http_request();
        let info = req.connection_info();
        assert_eq!(info.scheme(), "http");
        assert_eq!(info.host(), "localhost:8080");

        let req = TestRequest::default()
            .insert_header((
                header::FORWARDED,
                "for=192.0.2.60; proto=https; by=203.0.113.43; host=rust-lang.org",
            ))
            .to_http_request();

        let info = req.connection_info();
        assert_eq!(info.scheme(), "https");
        assert_eq!(info.host(), "rust-lang.org");
        assert_eq!(info.realip_remote_addr(), Some("192.0.2.60"));


Review comment (suggestion):
Is there any point in allowing users to construct this type? If not, I'd suggest making it `#[non_exhaustive]`.

Code after:
/// # Examples
/// ```
/// # use actix_web::Responder;
/// use actix_web::dev::PeerAddr;
///
/// async fn handler(peer_addr: PeerAddr) -> impl Responder {
///     let socket_addr = peer_addr.0;
///     socket_addr.to_string()
/// }
/// # let _svc = actix_web::web::to(handler);
/// ```
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord, Display)]
#[display(fmt = "{}", _0)]
pub struct PeerAddr(pub SocketAddr);

impl PeerAddr {
    /// Unwrap into inner `SocketAddr` value.
    pub fn into_inner(self) -> SocketAddr {
        self.0
    }
}

#[derive(Debug, Display, Error)]
#[non_exhaustive]
#[display(fmt = "Missing peer address")]
pub struct MissingPeerAddr;

impl ResponseError for MissingPeerAddr {}

impl FromRequest for PeerAddr {
    type Error = MissingPeerAddr;
    type Future = Ready<Result<Self, Self::Error>>;
    type Config = ();

    fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future {
        match req.peer_addr() {
            Some(addr) => ok(PeerAddr(addr)),
            None => {
                log::error!("Missing peer address.");
                err(MissingPeerAddr)
            }
        }
    }
}

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

    #[test]
Repository: actix/actix-web
File: src/request.rs (Rust)

Code before:

        if self.extensions().get::<Cookies>().is_none() {
            let mut cookies = Vec::new();
            for hdr in self.headers().get_all(COOKIE) {
                let s = str::from_utf8(hdr.as_bytes()).map_err(CookieParseError::from)?;
                for cookie_str in s.split(';').map(|s| s.trim()) {
                    if !cookie_str.is_empty() {
                        cookies.push(Cookie::parse_encoded(cookie_str)?.into_owned());
                    }
                }
            }
            self.extensions_mut().insert(Cookies(cookies));
        }

        Ok(Ref::map(self.extensions(), |ext| {
            &ext.get::<Cookies>().unwrap().0
        }))
    }

    /// Return request cookie.
    #[cfg(feature = "cookies")]
    pub fn cookie(&self, name: &str) -> Option<Cookie<'static>> {
        if let Ok(cookies) = self.cookies() {
            for cookie in cookies.iter() {
                if cookie.name() == name {
                    return Some(cookie.clone());
                }
            }
        }
        None
    }
}

impl HttpMessage for HttpRequest {
    type Stream = ();

    #[inline]
    /// Returns Request's headers.
    fn headers(&self) -> &HeaderMap {
        &self.head().headers
    }

    /// Request extensions
    #[inline]
    fn extensions(&self) -> Ref<'_, Extensions> {
        self.inner.head.extensions()
    }

    /// Mutable reference to a the request's extensions
    #[inline]
    fn extensions_mut(&self) -> RefMut<'_, Extensions> {

Review comment (suggestion):
```suggestion
                    return Some(cookie.to_owned());
```

Code after:

        if self.extensions().get::<Cookies>().is_none() {
            let mut cookies = Vec::new();
            for hdr in self.headers().get_all(COOKIE) {
                let s = str::from_utf8(hdr.as_bytes()).map_err(CookieParseError::from)?;
                for cookie_str in s.split(';').map(|s| s.trim()) {
                    if !cookie_str.is_empty() {
                        cookies.push(Cookie::parse_encoded(cookie_str)?.into_owned());
                    }
                }
            }
            self.extensions_mut().insert(Cookies(cookies));
        }

        Ok(Ref::map(self.extensions(), |ext| {
            &ext.get::<Cookies>().unwrap().0
        }))
    }

    /// Return request cookie.
    #[cfg(feature = "cookies")]
    pub fn cookie(&self, name: &str) -> Option<Cookie<'static>> {
        if let Ok(cookies) = self.cookies() {
            for cookie in cookies.iter() {
                if cookie.name() == name {
                    return Some(cookie.to_owned());
                }
            }
        }
        None
    }
}

impl HttpMessage for HttpRequest {
    type Stream = ();

    #[inline]
    /// Returns Request's headers.
    fn headers(&self) -> &HeaderMap {
        &self.head().headers
    }

    /// Request extensions
    #[inline]
    fn extensions(&self) -> Ref<'_, Extensions> {
        self.inner.head.extensions()
    }

    /// Mutable reference to a the request's extensions
    #[inline]
    fn extensions_mut(&self) -> RefMut<'_, Extensions> {