ronantakizawa/codereview-bench huggingface dataset:
Repository: tokio-rs/axum
File: axum/src/extract/connect_info.rs (Rust)

Code before:
        extract::connect_info::Connected, routing::get, serve::IncomingStream, serve::Listener,
        test_helpers::TestClient, Router,
    };
    use tokio::net::{TcpListener, TcpStream};

    #[test]
    fn into_make_service_traits() {
        use crate::test_helpers::*;
        assert_send::<IntoMakeServiceWithConnectInfo<(), NotSendSync>>();
    }

    #[allow(dead_code)]
    #[test]
    fn connected_traits() {
        // Test that the `Connected` trait can be used with custom address and listener types.

        fn create_router() -> Router {
            todo!()
        }

        fn tcp_listener() -> TcpListener {
            todo!()
        }

        #[derive(Clone)]
        #[allow(dead_code)]
        struct CustomAddr(SocketAddr);

        impl Connected<IncomingStream<'_, TcpListener>> for CustomAddr {
            fn connect_info(_stream: IncomingStream<'_, TcpListener>) -> Self {
                todo!()
            }
        }

        impl Connected<IncomingStream<'_, CustomListener>> for CustomAddr {
            fn connect_info(_stream: IncomingStream<'_, CustomListener>) -> Self {
                todo!()
            }
        }

        struct CustomListener {}

        impl Listener for CustomListener {
            type Io = TcpStream;
            type Addr = SocketAddr;

            async fn accept(&mut self) -> (Self::Io, Self::Addr) {
                todo!()
            }

            fn local_addr(&self) -> tokio::io::Result<Self::Addr> {

Review comment (suggestion):
```suggestion
```

It's already allowed from the higher level.

Code after:
        extract::connect_info::Connected, routing::get, serve::IncomingStream, serve::Listener,
        test_helpers::TestClient, Router,
    };
    use tokio::net::{TcpListener, TcpStream};

    #[test]
    fn into_make_service_traits() {
        use crate::test_helpers::*;
        assert_send::<IntoMakeServiceWithConnectInfo<(), NotSendSync>>();
    }

    #[allow(dead_code)]
    #[allow(clippy::todo)]
    fn connected_traits() {
        // Test that the `Connected` trait can be used with custom address and listener types.

        fn create_router() -> Router {
            todo!()
        }

        fn tcp_listener() -> TcpListener {
            todo!()
        }

        #[derive(Clone)]
        struct CustomAddr(SocketAddr);

        impl Connected<IncomingStream<'_, TcpListener>> for CustomAddr {
            fn connect_info(_stream: IncomingStream<'_, TcpListener>) -> Self {
                todo!()
            }
        }

        impl Connected<IncomingStream<'_, CustomListener>> for CustomAddr {
            fn connect_info(_stream: IncomingStream<'_, CustomListener>) -> Self {
                todo!()
            }
        }

        struct CustomListener {}

        impl Listener for CustomListener {
            type Io = TcpStream;
            type Addr = SocketAddr;

            async fn accept(&mut self) -> (Self::Io, Self::Addr) {
                todo!()
            }

            fn local_addr(&self) -> tokio::io::Result<Self::Addr> {
                todo!()
Repository: tokio-rs/axum
File: axum-extra/src/extract/cookie/mod.rs (Rust)

Code before:
///         ))
///     } else {
///         Err(StatusCode::UNAUTHORIZED)
///     }
/// }
///
/// async fn me(jar: CookieJar) -> Result<(), StatusCode> {
///     if let Some(session_id) = jar.get("session_id") {
///         // fetch and render user...
///         # Ok(())
///     } else {
///         Err(StatusCode::UNAUTHORIZED)
///     }
/// }
///
/// async fn authorize_and_create_session(token: &str) -> Option<String> {
///     // authorize the user and create a session...
///     # todo!()
/// }
///
/// let app = Router::new()
///     .route("/sessions", post(create_session))
///     .route("/me", get(me));
/// # let app: Router = app;
/// ```
#[must_use]
#[derive(Debug, Default, Clone)]
pub struct CookieJar {
    jar: cookie::CookieJar,
}

impl<S> FromRequestParts<S> for CookieJar
where
    S: Send + Sync,
{
    type Rejection = Infallible;

    async fn from_request_parts(parts: &mut Parts, _state: &S) -> Result<Self, Self::Rejection> {
        Ok(Self::from_headers(&parts.headers))
    }
}

fn cookies_from_request(headers: &HeaderMap) -> impl Iterator<Item = Cookie<'static>> + '_ {
    headers
        .get_all(COOKIE)
        .into_iter()
        .filter_map(|value| value.to_str().ok())
        .flat_map(|value| value.split(';'))
        .filter_map(|cookie| Cookie::parse_encoded(cookie.to_owned()).ok())
}


Review comment (suggestion):
```suggestion
#[must_use = "`CookieJar` should be returned as part of a `Response`, otherwise it does nothing."]
```
 This one can use a bit of clarity why it must be used if all `add` and `remove` methods and so on return `Self`.

And the same thing applies to the other jars.

Code after:
///         ))
///     } else {
///         Err(StatusCode::UNAUTHORIZED)
///     }
/// }
///
/// async fn me(jar: CookieJar) -> Result<(), StatusCode> {
///     if let Some(session_id) = jar.get("session_id") {
///         // fetch and render user...
///         # Ok(())
///     } else {
///         Err(StatusCode::UNAUTHORIZED)
///     }
/// }
///
/// async fn authorize_and_create_session(token: &str) -> Option<String> {
///     // authorize the user and create a session...
///     # todo!()
/// }
///
/// let app = Router::new()
///     .route("/sessions", post(create_session))
///     .route("/me", get(me));
/// # let app: Router = app;
/// ```
#[must_use = "`CookieJar` should be returned as part of a `Response`, otherwise it does nothing."]
#[derive(Debug, Default, Clone)]
pub struct CookieJar {
    jar: cookie::CookieJar,
}

impl<S> FromRequestParts<S> for CookieJar
where
    S: Send + Sync,
{
    type Rejection = Infallible;

    async fn from_request_parts(parts: &mut Parts, _state: &S) -> Result<Self, Self::Rejection> {
        Ok(Self::from_headers(&parts.headers))
    }
}

fn cookies_from_request(headers: &HeaderMap) -> impl Iterator<Item = Cookie<'static>> + '_ {
    headers
        .get_all(COOKIE)
        .into_iter()
        .filter_map(|value| value.to_str().ok())
        .flat_map(|value| value.split(';'))
        .filter_map(|cookie| Cookie::parse_encoded(cookie.to_owned()).ok())
}
Repository: tokio-rs/axum
File: axum/src/extract/connect_info.rs (Rust)

Code before:
#[cfg(all(feature = "tokio", any(feature = "http1", feature = "http2")))]
const _: () = {
    use crate::serve;
    use tokio::net::TcpListener;

    impl Connected<serve::IncomingStream<'_, TcpListener>> for SocketAddr {
        fn connect_info(stream: serve::IncomingStream<'_, TcpListener>) -> Self {
            *stream.remote_addr()
        }
    }

    impl<'a, L, F> Connected<serve::IncomingStream<'a, serve::TapIo<L, F>>> for L::Addr
    where
        L: serve::Listener,
        L::Addr: Clone + Sync + 'static,
        F: FnMut(&mut L::Io) + Send + 'static,
    {
        fn connect_info(stream: serve::IncomingStream<'a, serve::TapIo<L, F>>) -> Self {
            stream.remote_addr().clone()
        }
    }
};

#[allow(clippy::use_self)]
impl Connected<SocketAddr> for SocketAddr {
    fn connect_info(remote_addr: SocketAddr) -> Self {
        remote_addr
    }
}

impl<S, C, T> Service<T> for IntoMakeServiceWithConnectInfo<S, C>
where
    S: Clone,
    C: Connected<T>,
{
    type Response = AddExtension<S, ConnectInfo<C>>;
    type Error = Infallible;
    type Future = ResponseFuture<S, C>;

    #[inline]
    fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
        Poll::Ready(Ok(()))
    }

    fn call(&mut self, target: T) -> Self::Future {
        let connect_info = ConnectInfo(C::connect_info(target));
        let svc = Extension(connect_info).layer(self.svc.clone());
        ResponseFuture::new(ready(Ok(svc)))
    }
}


Review comment (suggestion):
```suggestion
impl Connected<SocketAddr> for SocketAddr {
    fn connect_info(remote_addr: Self) -> Self {
```

Code after:
#[cfg(all(feature = "tokio", any(feature = "http1", feature = "http2")))]
const _: () = {
    use crate::serve;
    use tokio::net::TcpListener;

    impl Connected<serve::IncomingStream<'_, TcpListener>> for SocketAddr {
        fn connect_info(stream: serve::IncomingStream<'_, TcpListener>) -> Self {
            *stream.remote_addr()
        }
    }

    impl<'a, L, F> Connected<serve::IncomingStream<'a, serve::TapIo<L, F>>> for L::Addr
    where
        L: serve::Listener,
        L::Addr: Clone + Sync + 'static,
        F: FnMut(&mut L::Io) + Send + 'static,
    {
        fn connect_info(stream: serve::IncomingStream<'a, serve::TapIo<L, F>>) -> Self {
            stream.remote_addr().clone()
        }
    }
};

impl Connected<Self> for SocketAddr {
    fn connect_info(remote_addr: Self) -> Self {
        remote_addr
    }
}

impl<S, C, T> Service<T> for IntoMakeServiceWithConnectInfo<S, C>
where
    S: Clone,
    C: Connected<T>,
{
    type Response = AddExtension<S, ConnectInfo<C>>;
    type Error = Infallible;
    type Future = ResponseFuture<S, C>;

    #[inline]
    fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
        Poll::Ready(Ok(()))
    }

    fn call(&mut self, target: T) -> Self::Future {
        let connect_info = ConnectInfo(C::connect_info(target));
        let svc = Extension(connect_info).layer(self.svc.clone());
        ResponseFuture::new(ready(Ok(svc)))
    }
}

opaque_future! {
Repository: tokio-rs/axum
File: axum/src/middleware/map_request.rs (Rust)

Code before:
    impl<B, E> Sealed<B> for Result<Request<B>, E> where E: IntoResponse {}
    impl<B> Sealed<B> for Request<B> {}
}

/// Trait implemented by types that can be returned from [`map_request`],
/// [`map_request_with_state`].
///
/// This trait is sealed such that it cannot be implemented outside this crate.
pub trait IntoMapRequestResult<B>: private::Sealed<B> {
    /// Perform the conversion.
    #[allow(clippy::result_large_err)]
    fn into_map_request_result(self) -> Result<Request<B>, Response>;
}

impl<B, E> IntoMapRequestResult<B> for Result<Request<B>, E>
where
    E: IntoResponse,
{
    fn into_map_request_result(self) -> Result<Request<B>, Response> {
        self.map_err(IntoResponse::into_response)
    }
}

impl<B> IntoMapRequestResult<B> for Request<B> {
    #[allow(clippy::use_self)]
    fn into_map_request_result(self) -> Result<Request<B>, Response> {
        Ok(self)
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::{routing::get, test_helpers::TestClient, Router};
    use http::{HeaderMap, StatusCode};

    #[crate::test]
    async fn works() {
        async fn add_header<B>(mut req: Request<B>) -> Request<B> {
            req.headers_mut().insert("x-foo", "foo".parse().unwrap());
            req
        }

        async fn handler(headers: HeaderMap) -> Response {
            headers["x-foo"]
                .to_str()
                .unwrap()
                .to_owned()
                .into_response()
        }


Review comment (suggestion):
```suggestion
    fn into_map_request_result(self) -> Result<Self, Response> {
```

Code after:
    impl<B, E> Sealed<B> for Result<Request<B>, E> where E: IntoResponse {}
    impl<B> Sealed<B> for Request<B> {}
}

/// Trait implemented by types that can be returned from [`map_request`],
/// [`map_request_with_state`].
///
/// This trait is sealed such that it cannot be implemented outside this crate.
pub trait IntoMapRequestResult<B>: private::Sealed<B> {
    /// Perform the conversion.
    #[allow(clippy::result_large_err)]
    fn into_map_request_result(self) -> Result<Request<B>, Response>;
}

impl<B, E> IntoMapRequestResult<B> for Result<Request<B>, E>
where
    E: IntoResponse,
{
    fn into_map_request_result(self) -> Result<Request<B>, Response> {
        self.map_err(IntoResponse::into_response)
    }
}

impl<B> IntoMapRequestResult<B> for Request<B> {
    fn into_map_request_result(self) -> Result<Self, Response> {
        Ok(self)
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::{routing::get, test_helpers::TestClient, Router};
    use http::{HeaderMap, StatusCode};

    #[crate::test]
    async fn works() {
        async fn add_header<B>(mut req: Request<B>) -> Request<B> {
            req.headers_mut().insert("x-foo", "foo".parse().unwrap());
            req
        }

        async fn handler(headers: HeaderMap) -> Response {
            headers["x-foo"]
                .to_str()
                .unwrap()
                .to_owned()
                .into_response()
        }

        let app = Router::new()
Repository: tokio-rs/axum
File: axum/src/extract/connect_info.rs (Rust)

Code before:
}

#[cfg(all(feature = "tokio", any(feature = "http1", feature = "http2")))]
const _: () = {
    use crate::serve;
    use tokio::net::TcpListener;

    impl Connected<serve::IncomingStream<'_, TcpListener>> for SocketAddr {
        fn connect_info(stream: serve::IncomingStream<'_, TcpListener>) -> Self {
            *stream.remote_addr()
        }
    }

    impl<'a, L, F> Connected<serve::IncomingStream<'a, serve::TapIo<L, F>>> for L::Addr
    where
        L: serve::Listener,
        L::Addr: Clone + Sync + 'static,
        F: FnMut(&mut L::Io) + Send + 'static,
    {
        fn connect_info(stream: serve::IncomingStream<'a, serve::TapIo<L, F>>) -> Self {
            stream.remote_addr().clone()
        }
    }
};

impl Connected<SocketAddr> for SocketAddr {
    fn connect_info(remote_addr: Self) -> Self {
        remote_addr
    }
}

impl<S, C, T> Service<T> for IntoMakeServiceWithConnectInfo<S, C>
where
    S: Clone,
    C: Connected<T>,
{
    type Response = AddExtension<S, ConnectInfo<C>>;
    type Error = Infallible;
    type Future = ResponseFuture<S, C>;

    #[inline]
    fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
        Poll::Ready(Ok(()))
    }

    fn call(&mut self, target: T) -> Self::Future {
        let connect_info = ConnectInfo(C::connect_info(target));
        let svc = Extension(connect_info).layer(self.svc.clone());
        ResponseFuture::new(ready(Ok(svc)))
    }
}

Review comment (suggestion):
```suggestion
impl Connected<Self> for SocketAddr {
```

Code after:
}

#[cfg(all(feature = "tokio", any(feature = "http1", feature = "http2")))]
const _: () = {
    use crate::serve;
    use tokio::net::TcpListener;

    impl Connected<serve::IncomingStream<'_, TcpListener>> for SocketAddr {
        fn connect_info(stream: serve::IncomingStream<'_, TcpListener>) -> Self {
            *stream.remote_addr()
        }
    }

    impl<'a, L, F> Connected<serve::IncomingStream<'a, serve::TapIo<L, F>>> for L::Addr
    where
        L: serve::Listener,
        L::Addr: Clone + Sync + 'static,
        F: FnMut(&mut L::Io) + Send + 'static,
    {
        fn connect_info(stream: serve::IncomingStream<'a, serve::TapIo<L, F>>) -> Self {
            stream.remote_addr().clone()
        }
    }
};

impl Connected<Self> for SocketAddr {
    fn connect_info(remote_addr: Self) -> Self {
        remote_addr
    }
}

impl<S, C, T> Service<T> for IntoMakeServiceWithConnectInfo<S, C>
where
    S: Clone,
    C: Connected<T>,
{
    type Response = AddExtension<S, ConnectInfo<C>>;
    type Error = Infallible;
    type Future = ResponseFuture<S, C>;

    #[inline]
    fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
        Poll::Ready(Ok(()))
    }

    fn call(&mut self, target: T) -> Self::Future {
        let connect_info = ConnectInfo(C::connect_info(target));
        let svc = Extension(connect_info).layer(self.svc.clone());
        ResponseFuture::new(ready(Ok(svc)))
    }
}
Repository: tokio-rs/axum
File: axum-extra/src/protobuf.rs (Rust)

Code before:
    ///
    /// Contains one variant for each way the [`Protobuf`] extractor
    /// can fail.
    pub enum ProtobufRejection {
        ProtobufDecodeError,
        BytesRejection,
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::test_helpers::*;
    use axum::{routing::post, Router};

    #[tokio::test]
    async fn decode_body() {
        #[derive(prost::Message)]
        struct Input {
            #[prost(string, tag = "1")]
            foo: String,
        }

        let app = Router::new().route(
            "/",
            post(|input: Protobuf<Input>| async move { input.0.foo }),
        );

        let input = Input {
            foo: "bar".to_owned(),
        };

        let client = TestClient::new(app);
        let res = client.post("/").body(input.encode_to_vec()).await;

        let body = res.text().await;

        assert_eq!(body, "bar");
    }

    #[tokio::test]
    async fn prost_decode_error() {
        #[derive(prost::Message)]
        struct Input {
            #[prost(string, tag = "1")]
            foo: String,
        }

        #[derive(prost::Message)]
        struct Expected {
            #[prost(int32, tag = "1")]

Review comment (suggestion):
```suggestion
            post(|Protobuf(input): Protobuf<Input>| async move { input.foo }),
```

Code after:
    ///
    /// Contains one variant for each way the [`Protobuf`] extractor
    /// can fail.
    pub enum ProtobufRejection {
        ProtobufDecodeError,
        BytesRejection,
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::test_helpers::*;
    use axum::{routing::post, Router};

    #[tokio::test]
    async fn decode_body() {
        #[derive(prost::Message)]
        struct Input {
            #[prost(string, tag = "1")]
            foo: String,
        }

        let app = Router::new().route(
            "/",
            post(|Protobuf(input): Protobuf<Input>| async move { input.foo }),
        );

        let input = Input {
            foo: "bar".to_owned(),
        };

        let client = TestClient::new(app);
        let res = client.post("/").body(input.encode_to_vec()).await;

        let body = res.text().await;

        assert_eq!(body, "bar");
    }

    #[tokio::test]
    async fn prost_decode_error() {
        #[derive(prost::Message)]
        struct Input {
            #[prost(string, tag = "1")]
            foo: String,
        }

        #[derive(prost::Message)]
        struct Expected {
            #[prost(int32, tag = "1")]
Repository: tokio-rs/axum
File: axum/src/response/sse.rs (Rust)

Code before:
        let buffer = self.event.buffer.as_mut();

        if !std::mem::replace(&mut self.data_written, true) {
            if self.event.flags.contains(EventFlags::HAS_DATA) {
                panic!("Called `Event::data*` multiple times");
            }

            let _ = buffer.write_str("data: ");
            self.event.flags.insert(EventFlags::HAS_DATA);
        }

        let mut writer = buffer.writer();

        let mut last_split = 0;
        for delimiter in memchr::memchr2_iter(b'\n', b'\r', buf) {
            let _ = writer.write_all(&buf[last_split..=delimiter]);
            let _ = writer.write_all(b"data: ");
            last_split = delimiter + 1;
        }
        let _ = writer.write_all(&buf[last_split..]);

        buf.len()
    }
}

impl std::fmt::Write for EventDataWriter {
    fn write_str(&mut self, s: &str) -> fmt::Result {
        let _ = self.write_buf(s.as_bytes());
        Ok(())
    }
}

impl Default for Event {
    fn default() -> Self {
        Self {
            buffer: Buffer::Active(BytesMut::new()),
            flags: EventFlags::from_bits(0),
        }
    }
}

#[derive(Debug, Copy, Clone, PartialEq)]
struct EventFlags(u8);

impl EventFlags {
    const HAS_DATA: Self = Self::from_bits(0b0001);
    const HAS_EVENT: Self = Self::from_bits(0b0010);
    const HAS_RETRY: Self = Self::from_bits(0b0100);
    const HAS_ID: Self = Self::from_bits(0b1000);

    const fn bits(self) -> u8 {

Review comment (suggestion):
```suggestion
impl fmt::Write for EventDataWriter {
```

Code after:
        let buffer = self.event.buffer.as_mut();

        if !std::mem::replace(&mut self.data_written, true) {
            if self.event.flags.contains(EventFlags::HAS_DATA) {
                panic!("Called `Event::data*` multiple times");
            }

            let _ = buffer.write_str("data: ");
            self.event.flags.insert(EventFlags::HAS_DATA);
        }

        let mut writer = buffer.writer();

        let mut last_split = 0;
        for delimiter in memchr::memchr2_iter(b'\n', b'\r', buf) {
            let _ = writer.write_all(&buf[last_split..=delimiter]);
            let _ = writer.write_all(b"data: ");
            last_split = delimiter + 1;
        }
        let _ = writer.write_all(&buf[last_split..]);

        buf.len()
    }
}

impl fmt::Write for EventDataWriter {
    fn write_str(&mut self, s: &str) -> fmt::Result {
        let _ = self.write_buf(s.as_bytes());
        Ok(())
    }
}

impl Default for Event {
    fn default() -> Self {
        Self {
            buffer: Buffer::Active(BytesMut::new()),
            flags: EventFlags::from_bits(0),
        }
    }
}

#[derive(Debug, Copy, Clone, PartialEq)]
struct EventFlags(u8);

impl EventFlags {
    const HAS_DATA: Self = Self::from_bits(0b0001);
    const HAS_EVENT: Self = Self::from_bits(0b0010);
    const HAS_RETRY: Self = Self::from_bits(0b0100);
    const HAS_ID: Self = Self::from_bits(0b1000);

    const fn bits(self) -> u8 {
Repository: tokio-rs/axum
File: examples/anyhow-error-response/src/main.rs (Rust)

Code before:

fn app() -> Router {
    Router::new().route("/", get(handler))
}

// This enables using `?` on functions that return `Result<_, anyhow::Error>` to turn them into
// `Result<_, AppError>`. That way you don't need to do that manually.
impl<E> From<E> for AppError
where
    E: Into<anyhow::Error>,
{
    fn from(err: E) -> Self {
        Self(err.into())
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    use axum::{body::Body, http::Request, http::StatusCode};
    use http_body_util::BodyExt;
    use tower::ServiceExt;

    #[tokio::test]
    async fn test_main_page() {
        let response = app().oneshot(Request::new(Body::empty())).await.unwrap();

        assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR);
        let body = response.into_body();
        let bytes = body.collect().await.unwrap().to_bytes();
        let html = String::from_utf8(bytes.to_vec()).unwrap();

        assert_eq!(html, "Something went wrong: it failed!");
    }
}


Review comment (suggestion):
I'd personally prefer staying explicit about the uri `"/"` rather than relying on default.

Code after:

fn app() -> Router {
    Router::new().route("/", get(handler))
}

// This enables using `?` on functions that return `Result<_, anyhow::Error>` to turn them into
// `Result<_, AppError>`. That way you don't need to do that manually.
impl<E> From<E> for AppError
where
    E: Into<anyhow::Error>,
{
    fn from(err: E) -> Self {
        Self(err.into())
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    use axum::{body::Body, http::Request, http::StatusCode};
    use http_body_util::BodyExt;
    use tower::ServiceExt;

    #[tokio::test]
    async fn test_main_page() {
        let response = app()
            .oneshot(Request::get("/").body(Body::empty()).unwrap())
            .await
            .unwrap();

        assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR);
        let body = response.into_body();
        let bytes = body.collect().await.unwrap().to_bytes();
        let html = String::from_utf8(bytes.to_vec()).unwrap();

        assert_eq!(html, "Something went wrong: it failed!");
    }
}