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

Code before:
///     middleware::map_request,
///     extract::Path,
///     http::Request,
/// };
/// use std::collections::HashMap;
///
/// async fn log_path_params<B>(
///     Path(path_params): Path<HashMap<String, String>>,
///     request: Request<B>,
/// ) -> Request<B> {
///     tracing::debug!(?path_params);
///     request
/// }
///
/// let app = Router::new()
///     .route("/", get(|| async { /* ... */ }))
///     .layer(map_request(log_path_params));
/// # let _: Router = app;
/// ```
///
/// Note that to access state you must use either [`map_request_with_state`] or [`map_request_with_state_arc`].
pub fn map_request<F, T>(f: F) -> MapRequestLayer<F, (), T> {
    map_request_with_state((), f)
}

/// Create a middleware from an async function, that transforms a request, with the given state.
///
/// See [`State`](crate::extract::State) for more details about accessing state.
///
/// # Example
///
/// ```rust
/// use axum::{
///     Router,
///     http::{Request, StatusCode},
///     routing::get,
///     response::IntoResponse,
///     middleware::map_request_with_state,
///     extract::State,
/// };
///
/// #[derive(Clone)]
/// struct AppState { /* ... */ }
///
/// async fn my_middleware<B>(
///     State(state): State<AppState>,
///     // you can add more extractors here...
///     request: Request<B>,
/// ) -> Request<B> {
///     // do something with `request`...
///     request

Review comment (suggestion):
```suggestion
/// Create a middleware from an async function that transforms a request, with the given state.
```

Code after:
///     middleware::map_request,
///     extract::Path,
///     http::Request,
/// };
/// use std::collections::HashMap;
///
/// async fn log_path_params<B>(
///     Path(path_params): Path<HashMap<String, String>>,
///     request: Request<B>,
/// ) -> Request<B> {
///     tracing::debug!(?path_params);
///     request
/// }
///
/// let app = Router::new()
///     .route("/", get(|| async { /* ... */ }))
///     .layer(map_request(log_path_params));
/// # let _: Router = app;
/// ```
///
/// Note that to access state you must use either [`map_request_with_state`] or [`map_request_with_state_arc`].
pub fn map_request<F, T>(f: F) -> MapRequestLayer<F, (), T> {
    map_request_with_state((), f)
}

/// Create a middleware from an async function that transforms a request, with the given state.
///
/// See [`State`](crate::extract::State) for more details about accessing state.
///
/// # Example
///
/// ```rust
/// use axum::{
///     Router,
///     http::{Request, StatusCode},
///     routing::get,
///     response::IntoResponse,
///     middleware::map_request_with_state,
///     extract::State,
/// };
///
/// #[derive(Clone)]
/// struct AppState { /* ... */ }
///
/// async fn my_middleware<B>(
///     State(state): State<AppState>,
///     // you can add more extractors here but the last
///     // extractor must implement `FromRequest` which
///     // `Request` does
///     request: Request<B>,
/// ) -> Request<B> {
Repository: tokio-rs/axum
File: axum/src/middleware/from_fn.rs (Rust)

Code before:
///
/// let app = Router::new()
///     .route("/", get(|| async { /* ... */ }))
///     .route_layer(middleware::from_fn(auth));
/// # let app: Router = app;
/// ```
///
/// # Running extractors
///
/// ```rust
/// use axum::{
///     Router,
///     extract::{TypedHeader, Query},
///     headers::authorization::{Authorization, Bearer},
///     http::Request,
///     middleware::{self, Next},
///     response::Response,
///     routing::get,
/// };
/// use std::collections::HashMap;
///
/// async fn my_middleware<B>(
///     TypedHeader(auth): TypedHeader<Authorization<Bearer>>,
///     Query(query_params): Query<HashMap<String, String>>,
///     // you can add more extractors here but the last
///     // extractor most implement `FromRequest` which
///     // `Request` does
///     req: Request<B>,
///     next: Next<B>,
/// ) -> Response {
///     // do something with `auth` and `query_params`...
///
///     next.run(req).await
/// }
///
/// let app = Router::new()
///     .route("/", get(|| async { /* ... */ }))
///     .route_layer(middleware::from_fn(my_middleware));
/// # let app: Router = app;
/// ```
///
/// [extractors]: crate::extract::FromRequest
pub fn from_fn<F, T>(f: F) -> FromFnLayer<F, (), T> {
    from_fn_with_state((), f)
}

/// Create a middleware from an async function with the given state.
///
/// See [`State`](crate::extract::State) for more details about accessing state.
///
/// # Example

Review comment (refactor):
```suggestion
///     // extractor must implement `FromRequest` which
```

Code after:
///
/// let app = Router::new()
///     .route("/", get(|| async { /* ... */ }))
///     .route_layer(middleware::from_fn(auth));
/// # let app: Router = app;
/// ```
///
/// # Running extractors
///
/// ```rust
/// use axum::{
///     Router,
///     extract::{TypedHeader, Query},
///     headers::authorization::{Authorization, Bearer},
///     http::Request,
///     middleware::{self, Next},
///     response::Response,
///     routing::get,
/// };
/// use std::collections::HashMap;
///
/// async fn my_middleware<B>(
///     TypedHeader(auth): TypedHeader<Authorization<Bearer>>,
///     Query(query_params): Query<HashMap<String, String>>,
///     // you can add more extractors here but the last
///     // extractor must implement `FromRequest` which
///     // `Request` does
///     req: Request<B>,
///     next: Next<B>,
/// ) -> Response {
///     // do something with `auth` and `query_params`...
///
///     next.run(req).await
/// }
///
/// let app = Router::new()
///     .route("/", get(|| async { /* ... */ }))
///     .route_layer(middleware::from_fn(my_middleware));
/// # let app: Router = app;
/// ```
///
/// [extractors]: crate::extract::FromRequest
pub fn from_fn<F, T>(f: F) -> FromFnLayer<F, (), T> {
    from_fn_with_state((), f)
}

/// Create a middleware from an async function with the given state.
///
/// See [`State`](crate::extract::State) for more details about accessing state.
///
/// # Example
Repository: tokio-rs/axum
File: axum/src/middleware/from_fn.rs (Rust)

Code before:
    from_fn_with_state((), f)
}

/// Create a middleware from an async function with the given state.
///
/// See [`State`](crate::extract::State) for more details about accessing state.
///
/// # Example
///
/// ```rust
/// use axum::{
///     Router,
///     http::{Request, StatusCode},
///     routing::get,
///     response::{IntoResponse, Response},
///     middleware::{self, Next},
///     extract::State,
/// };
///
/// #[derive(Clone)]
/// struct AppState { /* ... */ }
///
/// async fn my_middleware<B>(
///     State(state): State<AppState>,
///     // you can add more extractors here but the last
///     // extractor most implement `FromRequest` which
///     // `Request` does
///     req: Request<B>,
///     next: Next<B>,
/// ) -> Response {
///     // do something with `req`...
///     let res = next.run(req).await;
///     // do something with `res`...
///     res
/// }
///
/// let state = AppState { /* ... */ };
///
/// let app = Router::with_state(state.clone())
///     .route("/", get(|| async { /* ... */ }))
///     .route_layer(middleware::from_fn_with_state(state, my_middleware));
/// # let app: Router<_> = app;
/// ```
pub fn from_fn_with_state<F, S, T>(state: S, f: F) -> FromFnLayer<F, S, T> {
    from_fn_with_state_arc(Arc::new(state), f)
}

/// Create a middleware from an async function with the given [`Arc`]'ed state.
///
/// See [`from_fn_with_state`] for an example.
///

Review comment (refactor):
```suggestion
///     // extractor must implement `FromRequest` which
```

Code after:
    from_fn_with_state((), f)
}

/// Create a middleware from an async function with the given state.
///
/// See [`State`](crate::extract::State) for more details about accessing state.
///
/// # Example
///
/// ```rust
/// use axum::{
///     Router,
///     http::{Request, StatusCode},
///     routing::get,
///     response::{IntoResponse, Response},
///     middleware::{self, Next},
///     extract::State,
/// };
///
/// #[derive(Clone)]
/// struct AppState { /* ... */ }
///
/// async fn my_middleware<B>(
///     State(state): State<AppState>,
///     // you can add more extractors here but the last
///     // extractor must implement `FromRequest` which
///     // `Request` does
///     req: Request<B>,
///     next: Next<B>,
/// ) -> Response {
///     // do something with `req`...
///     let res = next.run(req).await;
///     // do something with `res`...
///     res
/// }
///
/// let state = AppState { /* ... */ };
///
/// let app = Router::with_state(state.clone())
///     .route("/", get(|| async { /* ... */ }))
///     .route_layer(middleware::from_fn_with_state(state, my_middleware));
/// # let app: Router<_> = app;
/// ```
pub fn from_fn_with_state<F, S, T>(state: S, f: F) -> FromFnLayer<F, S, T> {
    from_fn_with_state_arc(Arc::new(state), f)
}

/// Create a middleware from an async function with the given [`Arc`]'ed state.
///
/// See [`from_fn_with_state`] for an example.
///
Repository: tokio-rs/axum
File: axum-macros/src/from_ref.rs (Rust)

Code before:
use proc_macro2::{Ident, TokenStream};
use quote::{quote, quote_spanned};
use syn::{spanned::Spanned, Field, ItemStruct};

pub(crate) fn expand(item: ItemStruct) -> syn::Result<TokenStream> {
    let from_ref_impls = item
        .fields
        .iter()
        .enumerate()
        .map(|(idx, field)| expand_field(&item.ident, idx, field))
        .map(|result| match result {
            Ok(tokens) => tokens,
            Err(err) => err.into_compile_error(),
        });

    Ok(quote! {
        #(#from_ref_impls)*
    })
}

fn expand_field(state: &Ident, idx: usize, field: &Field) -> syn::Result<TokenStream> {
    let field_ty = &field.ty;
    let span = field.ty.span();

    let body = if let Some(field_ident) = &field.ident {
        quote_spanned! {span=> state.#field_ident.clone() }
    } else {
        let idx = syn::Index {
            index: idx as _,
            span: field.span(),

Review comment (suggestion):
```suggestion
pub(crate) fn expand(item: ItemStruct) -> TokenStream {
```

Code after:
use proc_macro2::{Ident, TokenStream};
use quote::quote_spanned;
use syn::{spanned::Spanned, Field, ItemStruct};

pub(crate) fn expand(item: ItemStruct) -> TokenStream {
    item.fields
        .iter()
        .enumerate()
        .map(|(idx, field)| expand_field(&item.ident, idx, field))
        .collect()
}

fn expand_field(state: &Ident, idx: usize, field: &Field) -> TokenStream {
    let field_ty = &field.ty;
    let span = field.ty.span();

    let body = if let Some(field_ident) = &field.ident {
        quote_spanned! {span=> state.#field_ident.clone() }
    } else {
        let idx = syn::Index {
            index: idx as _,
            span: field.span(),
        };
        quote_spanned! {span=> state.#idx.clone() }
    };

    quote_spanned! {span=>
        impl ::axum::extract::FromRef<#state> for #field_ty {
            fn from_ref(state: &#state) -> Self {
                #body
Repository: tokio-rs/axum
File: axum-macros/src/from_ref.rs (Rust)

Code before:
use proc_macro2::{Ident, TokenStream};
use quote::{quote, quote_spanned};
use syn::{spanned::Spanned, Field, ItemStruct};

pub(crate) fn expand(item: ItemStruct) -> syn::Result<TokenStream> {
    let from_ref_impls = item
        .fields
        .iter()
        .enumerate()
        .map(|(idx, field)| expand_field(&item.ident, idx, field))
        .map(|result| match result {
            Ok(tokens) => tokens,
            Err(err) => err.into_compile_error(),
        });

    Ok(quote! {
        #(#from_ref_impls)*
    })
}

fn expand_field(state: &Ident, idx: usize, field: &Field) -> syn::Result<TokenStream> {
    let field_ty = &field.ty;
    let span = field.ty.span();

    let body = if let Some(field_ident) = &field.ident {
        quote_spanned! {span=> state.#field_ident.clone() }
    } else {
        let idx = syn::Index {
            index: idx as _,
            span: field.span(),
        };
        quote_spanned! {span=> state.#idx.clone() }
    };

    Ok(quote_spanned! {span=>
        impl ::axum::extract::FromRef<#state> for #field_ty {
            fn from_ref(state: &#state) -> Self {
                #body
            }
        }
    })
}


Review comment (suggestion):
```suggestion
        })
        .collect()
```

Code after:
use proc_macro2::{Ident, TokenStream};
use quote::quote_spanned;
use syn::{spanned::Spanned, Field, ItemStruct};

pub(crate) fn expand(item: ItemStruct) -> TokenStream {
    item.fields
        .iter()
        .enumerate()
        .map(|(idx, field)| expand_field(&item.ident, idx, field))
        .collect()
}

fn expand_field(state: &Ident, idx: usize, field: &Field) -> TokenStream {
    let field_ty = &field.ty;
    let span = field.ty.span();

    let body = if let Some(field_ident) = &field.ident {
        quote_spanned! {span=> state.#field_ident.clone() }
    } else {
        let idx = syn::Index {
            index: idx as _,
            span: field.span(),
        };
        quote_spanned! {span=> state.#idx.clone() }
    };

    quote_spanned! {span=>
        impl ::axum::extract::FromRef<#state> for #field_ty {
            fn from_ref(state: &#state) -> Self {
                #body
            }
        }
    }
}

#[test]
fn ui() {
    crate::run_ui_tests("from_ref");
}
Repository: tokio-rs/axum
File: examples/prometheus-metrics/src/main.rs (Rust)

Code before:
//!
//! ```not_rust
//! cd examples && cargo run -p example-prometheus-metrics
//! ```

use axum::{
    extract::MatchedPath,
    http::Request,
    middleware::{self, Next},
    response::IntoResponse,
    routing::get,
    Router,
};
use metrics_exporter_prometheus::{Matcher, PrometheusBuilder, PrometheusHandle};
use std::{
    future::ready,
    net::SocketAddr,
    time::{Duration, Instant},
};
use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt};

fn metrics_app() -> Router {
    let recorder_handle = setup_metrics_recorder();
    Router::new()
        .route("/metrics", get(move || ready(recorder_handle.render())))
        .route_layer(middleware::from_fn(track_metrics))
}

fn main_app() -> Router {
    Router::new()
        .route("/fast", get(|| async {}))
        .route(
            "/slow",
            get(|| async {
                tokio::time::sleep(Duration::from_secs(1)).await;
            }),
        )
        .route_layer(middleware::from_fn(track_metrics))
}

async fn start_main_server() {
    let app = main_app();

    let addr = SocketAddr::from(([127, 0, 0, 1], 3000));
    tracing::debug!("listening on {}", addr);
    axum::Server::bind(&addr)
        .serve(app.into_make_service())
        .await
        .unwrap()
}


Review comment (suggestion):
Probably don't need to track metrics on the metrics server :P

```suggestion
```

Code after:
//!
//! ```not_rust
//! cd examples && cargo run -p example-prometheus-metrics
//! ```

use axum::{
    extract::MatchedPath,
    http::Request,
    middleware::{self, Next},
    response::IntoResponse,
    routing::get,
    Router,
};
use metrics_exporter_prometheus::{Matcher, PrometheusBuilder, PrometheusHandle};
use std::{
    future::ready,
    net::SocketAddr,
    time::{Duration, Instant},
};
use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt};

fn metrics_app() -> Router {
    let recorder_handle = setup_metrics_recorder();
    Router::new().route("/metrics", get(move || ready(recorder_handle.render())))
}

fn main_app() -> Router {
    Router::new()
        .route("/fast", get(|| async {}))
        .route(
            "/slow",
            get(|| async {
                tokio::time::sleep(Duration::from_secs(1)).await;
            }),
        )
        .route_layer(middleware::from_fn(track_metrics))
}

async fn start_main_server() {
    let app = main_app();

    let addr = SocketAddr::from(([127, 0, 0, 1], 3000));
    tracing::debug!("listening on {}", addr);
    axum::Server::bind(&addr)
        .serve(app.into_make_service())
        .await
        .unwrap()
}

async fn start_metrics_server() {
    let app = metrics_app();
Repository: tokio-rs/axum
File: axum/src/extract/matched_path.rs (Rust)

Code before:
/// # Matched path in nested routers
///
/// Because of how [nesting] works `MatchedPath` isn't accessible in middleware on nested routes:
///
/// ```
/// use axum::{
///     Router,
///     RequestExt,
///     routing::get,
///     extract::{MatchedPath, rejection::MatchedPathRejection},
///     middleware::map_request,
///     http::Request,
///     body::Body,
/// };
///
/// async fn access_matched_path(mut request: Request<Body>) -> Request<Body> {
///     // if `/foo/bar` is called this will be `Err(_)` since that matches
///     // a nested route
///     let matched_path: Result<MatchedPath, MatchedPathRejection> =
///         request.extract_parts::<MatchedPath>().await;
///
///     request
/// }
///
/// // `MatchedPath` is always accessible on handlers regardless
/// // if its for a nested route or not
/// async fn handler(matched_path: MatchedPath) {}
///
/// let app = Router::new()
///     .nest(
///         "/foo",
///         Router::new().route("/bar", get(handler)),
///     )
///     .layer(map_request(access_matched_path));
/// # let _: Router = app;
/// ```
///
/// [nesting]: crate::Router::nest
#[cfg_attr(docsrs, doc(cfg(feature = "matched-path")))]
#[derive(Clone, Debug)]
pub struct MatchedPath(pub(crate) Arc<str>);

impl MatchedPath {
    /// Returns a `str` representation of the path.
    pub fn as_str(&self) -> &str {
        &*self.0
    }
}

#[async_trait]
impl<S> FromRequestParts<S> for MatchedPath

Review comment (suggestion):
```suggestion
/// // of whether it's for a nested route or not
```

Code after:
/// # Matched path in nested routers
///
/// Because of how [nesting] works `MatchedPath` isn't accessible in middleware on nested routes:
///
/// ```
/// use axum::{
///     Router,
///     RequestExt,
///     routing::get,
///     extract::{MatchedPath, rejection::MatchedPathRejection},
///     middleware::map_request,
///     http::Request,
///     body::Body,
/// };
///
/// async fn access_matched_path(mut request: Request<Body>) -> Request<Body> {
///     // if `/foo/bar` is called this will be `Err(_)` since that matches
///     // a nested route
///     let matched_path: Result<MatchedPath, MatchedPathRejection> =
///         request.extract_parts::<MatchedPath>().await;
///
///     request
/// }
///
/// // `MatchedPath` is always accessible on handlers added via `Router::route`
/// async fn handler(matched_path: MatchedPath) {}
///
/// let app = Router::new()
///     .nest(
///         "/foo",
///         Router::new().route("/bar", get(handler)),
///     )
///     .layer(map_request(access_matched_path));
/// # let _: Router = app;
/// ```
///
/// [nesting]: crate::Router::nest
#[cfg_attr(docsrs, doc(cfg(feature = "matched-path")))]
#[derive(Clone, Debug)]
pub struct MatchedPath(pub(crate) Arc<str>);

impl MatchedPath {
    /// Returns a `str` representation of the path.
    pub fn as_str(&self) -> &str {
        &self.0
    }
}

#[async_trait]
impl<S> FromRequestParts<S> for MatchedPath
where
Repository: tokio-rs/axum
File: axum/src/extract/raw_form.rs (Rust)

Code before:
    has_content_type,
    rejection::{InvalidFormContentType, RawFormRejection},
};

use crate::{body::HttpBody, BoxError};

/// Extractor that extracts the raw form string, without parsing it.
///
/// # Example
///
/// ```rust,no_run
/// use axum::{
///     extract::RawForm,
///     routing::get,
///     Router
/// };
///
/// async fn handler(RawForm(form): RawForm) {}
///
/// let app = Router::new().route("/", get(handler));
/// # async {
/// # axum::Server::bind(&"".parse().unwrap()).serve(app.into_make_service()).await.unwrap();
/// # };
/// ```
#[derive(Debug)]
pub struct RawForm(pub Option<String>);

#[async_trait]
impl<S, B> FromRequest<S, B> for RawForm
where
    B: HttpBody + Send + 'static,
    B::Data: Send,
    B::Error: Into<BoxError>,
    S: Send + Sync,
{
    type Rejection = RawFormRejection;

    async fn from_request(req: Request<B>, state: &S) -> Result<Self, Self::Rejection> {
        if req.method() == Method::GET {
            Ok(Self(req.uri().query().map(String::from)))
        } else {
            if !has_content_type(req.headers(), &mime::APPLICATION_WWW_FORM_URLENCODED) {
                return Err(InvalidFormContentType.into());
            }

            Ok(Self(Some(String::from_request(req, state).await?)))
        }
    }
}


Review comment (bug):
I think using an `Option` here might give the impression that it'll `None` instead of returning a rejection if something fails.

I'm wondering if we should just use an empty string in the case there is no query and remove the Option. What do you think?

Code after:
use super::{
    has_content_type,
    rejection::{InvalidFormContentType, RawFormRejection},
};

use crate::{body::HttpBody, BoxError};

/// Extractor that extracts raw form requests.
///
/// For `GET` requests it will extract the raw query. For other methods it extracts the raw
/// `application/x-www-form-urlencoded` encoded request body.
///
/// # Example
///
/// ```rust,no_run
/// use axum::{
///     extract::RawForm,
///     routing::get,
///     Router
/// };
///
/// async fn handler(RawForm(form): RawForm) {}
///
/// let app = Router::new().route("/", get(handler));
/// # async {
/// # axum::Server::bind(&"".parse().unwrap()).serve(app.into_make_service()).await.unwrap();
/// # };
/// ```
#[derive(Debug)]
pub struct RawForm(pub Bytes);

#[async_trait]
impl<S, B> FromRequest<S, B> for RawForm
where
    B: HttpBody + Send + 'static,
    B::Data: Send,
    B::Error: Into<BoxError>,
    S: Send + Sync,
{
    type Rejection = RawFormRejection;

    async fn from_request(req: Request<B>, state: &S) -> Result<Self, Self::Rejection> {
        if req.method() == Method::GET {
            let mut bytes = BytesMut::new();

            if let Some(query) = req.uri().query() {
                bytes.extend(query.as_bytes());
            }

            Ok(Self(bytes.freeze()))
        } else {