From c5907747ad29d82c89b116a82a0aa3c214315fdf Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Tue, 19 Nov 2019 20:05:16 -0800 Subject: [PATCH] Remove implementation of Responder for (). Fixes #1108. 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. --- CHANGES.md | 7 +++++++ src/app.rs | 5 +++-- src/data.rs | 5 +++-- src/resource.rs | 8 ++++---- src/responder.rs | 13 ------------- src/scope.rs | 5 +++-- 6 files changed, 20 insertions(+), 23 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index bb17a7ef..3d4b2d78 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -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 diff --git a/src/app.rs b/src/app.rs index d9ac8c09..a9dc3f29 100644 --- a/src/app.rs +++ b/src/app.rs @@ -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, /// } /// - /// async fn index(data: web::Data) { + /// async fn index(data: web::Data) -> impl Responder { /// data.counter.set(data.counter.get() + 1); + /// HttpResponse::Ok() /// } /// /// fn main() { diff --git a/src/data.rs b/src/data.rs index a026946a..5ace3a8f 100644 --- a/src/data.rs +++ b/src/data.rs @@ -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` extractor to access data in handler. -/// async fn index(data: web::Data>) { +/// async fn index(data: web::Data>) -> impl Responder { /// let mut data = data.lock().unwrap(); /// data.counter += 1; +/// HttpResponse::Ok() /// } /// /// fn main() { diff --git a/src/resource.rs b/src/resource.rs index a06530d4..758e2f28 100644 --- a/src/resource.rs +++ b/src/resource.rs @@ -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( @@ -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); diff --git a/src/responder.rs b/src/responder.rs index b254567d..fd86bb68 100644 --- a/src/responder.rs +++ b/src/responder.rs @@ -131,15 +131,6 @@ impl Responder for ResponseBuilder { } } -impl Responder for () { - type Error = Error; - type Future = Ready>; - - fn respond_to(self, _: &HttpRequest) -> Self::Future { - ok(Response::build(StatusCode::OK).finish()) - } -} - impl Responder for (T, StatusCode) where T: Responder, @@ -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"); diff --git a/src/scope.rs b/src/scope.rs index e5c04d71..9bec0a1f 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -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, /// } /// - /// async fn index(data: web::Data) { + /// async fn index(data: web::Data) -> impl Responder { /// data.counter.set(data.counter.get() + 1); + /// HttpResponse::Ok() /// } /// /// fn main() {