Skip to content

Commit

Permalink
Remove implementation of Responder for (). Fixes actix#1108.
Browse files Browse the repository at this point in the history
Rationale:

- In Rust, one can omit a semicolon after a function's final expression to make
  its value the function's return value. It's common for people to include a
  semicolon after the last expression by mistake - common enough that the Rust
  compiler suggests removing the semicolon when there's a type mismatch between
  the function's signature and body. By implementing Responder for (), Actix makes
  this common mistake a silent error in handler functions.

- Functions returning an empty body should return HTTP status 204 ("No Content"),
  so the current Responder impl for (), which returns status 200 ("OK"), is not
  really what one wants anyway.

- It's not much of a burden to ask handlers to explicitly return
  `HttpResponse::Ok()` if that is what they want; all the examples in the
  documentation do this already.
  • Loading branch information
jimblandy authored and fafhrd91 committed Nov 23, 2019
1 parent 525c22d commit c590774
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 23 deletions.
7 changes: 7 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# Changes

## [2.0.0-alpha.2] - 2019-xx-xx

### Changed

* Remove implementation of `Responder` for `()`. (#1167)


## [1.0.9] - 2019-11-14

### Added
Expand Down
5 changes: 3 additions & 2 deletions src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,15 @@ where
///
/// ```rust
/// use std::cell::Cell;
/// use actix_web::{web, App};
/// use actix_web::{web, App, HttpResponse, Responder};
///
/// struct MyData {
/// counter: Cell<usize>,
/// }
///
/// async fn index(data: web::Data<MyData>) {
/// async fn index(data: web::Data<MyData>) -> impl Responder {
/// data.counter.set(data.counter.get() + 1);
/// HttpResponse::Ok()
/// }
///
/// fn main() {
Expand Down
5 changes: 3 additions & 2 deletions src/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,17 @@ pub(crate) trait DataFactory {
///
/// ```rust
/// use std::sync::Mutex;
/// use actix_web::{web, App};
/// use actix_web::{web, App, HttpResponse, Responder};
///
/// struct MyData {
/// counter: usize,
/// }
///
/// /// Use `Data<T>` extractor to access data in handler.
/// async fn index(data: web::Data<Mutex<MyData>>) {
/// async fn index(data: web::Data<Mutex<MyData>>) -> impl Responder {
/// let mut data = data.lock().unwrap();
/// data.counter += 1;
/// HttpResponse::Ok()
/// }
///
/// fn main() {
Expand Down
8 changes: 4 additions & 4 deletions src/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ where
/// match guards for route selection.
///
/// ```rust
/// use actix_web::{web, guard, App, HttpResponse};
/// use actix_web::{web, guard, App};
///
/// fn main() {
/// let app = App::new().service(
Expand All @@ -156,9 +156,9 @@ where
/// .route(web::delete().to(delete_handler))
/// );
/// }
/// # async fn get_handler() -> impl actix_web::Responder { HttpResponse::Ok() }
/// # async fn post_handler() -> impl actix_web::Responder { HttpResponse::Ok() }
/// # async fn delete_handler() -> impl actix_web::Responder { HttpResponse::Ok() }
/// # async fn get_handler() -> impl actix_web::Responder { actix_web::HttpResponse::Ok() }
/// # async fn post_handler() -> impl actix_web::Responder { actix_web::HttpResponse::Ok() }
/// # async fn delete_handler() -> impl actix_web::Responder { actix_web::HttpResponse::Ok() }
/// ```
pub fn route(mut self, route: Route) -> Self {
self.routes.push(route);
Expand Down
13 changes: 0 additions & 13 deletions src/responder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,15 +131,6 @@ impl Responder for ResponseBuilder {
}
}

impl Responder for () {
type Error = Error;
type Future = Ready<Result<Response, Error>>;

fn respond_to(self, _: &HttpRequest) -> Self::Future {
ok(Response::build(StatusCode::OK).finish())
}
}

impl<T> Responder for (T, StatusCode)
where
T: Responder,
Expand Down Expand Up @@ -530,10 +521,6 @@ pub(crate) mod tests {
block_on(async {
let req = TestRequest::default().to_http_request();

let resp: HttpResponse = ().respond_to(&req).await.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
assert_eq!(*resp.body().body(), Body::Empty);

let resp: HttpResponse = "test".respond_to(&req).await.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
assert_eq!(resp.body().bin_ref(), b"test");
Expand Down
5 changes: 3 additions & 2 deletions src/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,14 +126,15 @@ where
///
/// ```rust
/// use std::cell::Cell;
/// use actix_web::{web, App};
/// use actix_web::{web, App, HttpResponse, Responder};
///
/// struct MyData {
/// counter: Cell<usize>,
/// }
///
/// async fn index(data: web::Data<MyData>) {
/// async fn index(data: web::Data<MyData>) -> impl Responder {
/// data.counter.set(data.counter.get() + 1);
/// HttpResponse::Ok()
/// }
///
/// fn main() {
Expand Down

0 comments on commit c590774

Please sign in to comment.