Skip to content

Commit 78b7d49

Browse files
committed
Auto merge of #5576 - ebroto:manual_async_fn, r=flip1995
Add the manual_async_fn lint changelog: Added the `manual_async_fn` lint to warn on functions that can be simplified using async syntax Closes #5503
2 parents b63868d + 3e4bc02 commit 78b7d49

10 files changed

+421
-3
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -1422,6 +1422,7 @@ Released 2018-09-13
14221422
[`lossy_float_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#lossy_float_literal
14231423
[`macro_use_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#macro_use_imports
14241424
[`main_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#main_recursion
1425+
[`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn
14251426
[`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy
14261427
[`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
14271428
[`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic

clippy_lints/src/lib.rs

+5
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@ mod literal_representation;
247247
mod loops;
248248
mod macro_use;
249249
mod main_recursion;
250+
mod manual_async_fn;
250251
mod manual_non_exhaustive;
251252
mod map_clone;
252253
mod map_unit_fn;
@@ -629,6 +630,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
629630
&loops::WHILE_LET_ON_ITERATOR,
630631
&macro_use::MACRO_USE_IMPORTS,
631632
&main_recursion::MAIN_RECURSION,
633+
&manual_async_fn::MANUAL_ASYNC_FN,
632634
&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE,
633635
&map_clone::MAP_CLONE,
634636
&map_unit_fn::OPTION_MAP_UNIT_FN,
@@ -1067,6 +1069,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10671069
store.register_late_pass(|| box if_let_mutex::IfLetMutex);
10681070
store.register_late_pass(|| box match_on_vec_items::MatchOnVecItems);
10691071
store.register_early_pass(|| box manual_non_exhaustive::ManualNonExhaustive);
1072+
store.register_late_pass(|| box manual_async_fn::ManualAsyncFn);
10701073

10711074
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
10721075
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1284,6 +1287,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
12841287
LintId::of(&loops::WHILE_LET_LOOP),
12851288
LintId::of(&loops::WHILE_LET_ON_ITERATOR),
12861289
LintId::of(&main_recursion::MAIN_RECURSION),
1290+
LintId::of(&manual_async_fn::MANUAL_ASYNC_FN),
12871291
LintId::of(&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
12881292
LintId::of(&map_clone::MAP_CLONE),
12891293
LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN),
@@ -1478,6 +1482,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
14781482
LintId::of(&loops::NEEDLESS_RANGE_LOOP),
14791483
LintId::of(&loops::WHILE_LET_ON_ITERATOR),
14801484
LintId::of(&main_recursion::MAIN_RECURSION),
1485+
LintId::of(&manual_async_fn::MANUAL_ASYNC_FN),
14811486
LintId::of(&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
14821487
LintId::of(&map_clone::MAP_CLONE),
14831488
LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH),

clippy_lints/src/manual_async_fn.rs

+159
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
use crate::utils::paths::FUTURE_FROM_GENERATOR;
2+
use crate::utils::{match_function_call, snippet_block, snippet_opt, span_lint_and_then};
3+
use if_chain::if_chain;
4+
use rustc_errors::Applicability;
5+
use rustc_hir::intravisit::FnKind;
6+
use rustc_hir::{
7+
AsyncGeneratorKind, Block, Body, Expr, ExprKind, FnDecl, FnRetTy, GeneratorKind, GenericBound, HirId, IsAsync,
8+
ItemKind, TraitRef, Ty, TyKind, TypeBindingKind,
9+
};
10+
use rustc_lint::{LateContext, LateLintPass};
11+
use rustc_session::{declare_lint_pass, declare_tool_lint};
12+
use rustc_span::Span;
13+
14+
declare_clippy_lint! {
15+
/// **What it does:** It checks for manual implementations of `async` functions.
16+
///
17+
/// **Why is this bad?** It's more idiomatic to use the dedicated syntax.
18+
///
19+
/// **Known problems:** None.
20+
///
21+
/// **Example:**
22+
///
23+
/// ```rust
24+
/// use std::future::Future;
25+
///
26+
/// fn foo() -> impl Future<Output = i32> { async { 42 } }
27+
/// ```
28+
/// Use instead:
29+
/// ```rust
30+
/// use std::future::Future;
31+
///
32+
/// async fn foo() -> i32 { 42 }
33+
/// ```
34+
pub MANUAL_ASYNC_FN,
35+
style,
36+
"manual implementations of `async` functions can be simplified using the dedicated syntax"
37+
}
38+
39+
declare_lint_pass!(ManualAsyncFn => [MANUAL_ASYNC_FN]);
40+
41+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ManualAsyncFn {
42+
fn check_fn(
43+
&mut self,
44+
cx: &LateContext<'a, 'tcx>,
45+
kind: FnKind<'tcx>,
46+
decl: &'tcx FnDecl<'_>,
47+
body: &'tcx Body<'_>,
48+
span: Span,
49+
_: HirId,
50+
) {
51+
if_chain! {
52+
if let Some(header) = kind.header();
53+
if let IsAsync::NotAsync = header.asyncness;
54+
// Check that this function returns `impl Future`
55+
if let FnRetTy::Return(ret_ty) = decl.output;
56+
if let Some(trait_ref) = future_trait_ref(cx, ret_ty);
57+
if let Some(output) = future_output_ty(trait_ref);
58+
// Check that the body of the function consists of one async block
59+
if let ExprKind::Block(block, _) = body.value.kind;
60+
if block.stmts.is_empty();
61+
if let Some(closure_body) = desugared_async_block(cx, block);
62+
then {
63+
let header_span = span.with_hi(ret_ty.span.hi());
64+
65+
span_lint_and_then(
66+
cx,
67+
MANUAL_ASYNC_FN,
68+
header_span,
69+
"this function can be simplified using the `async fn` syntax",
70+
|diag| {
71+
if_chain! {
72+
if let Some(header_snip) = snippet_opt(cx, header_span);
73+
if let Some(ret_pos) = header_snip.rfind("->");
74+
if let Some((ret_sugg, ret_snip)) = suggested_ret(cx, output);
75+
then {
76+
let help = format!("make the function `async` and {}", ret_sugg);
77+
diag.span_suggestion(
78+
header_span,
79+
&help,
80+
format!("async {}{}", &header_snip[..ret_pos], ret_snip),
81+
Applicability::MachineApplicable
82+
);
83+
84+
let body_snip = snippet_block(cx, closure_body.value.span, "..", Some(block.span));
85+
diag.span_suggestion(
86+
block.span,
87+
"move the body of the async block to the enclosing function",
88+
body_snip.to_string(),
89+
Applicability::MachineApplicable
90+
);
91+
}
92+
}
93+
},
94+
);
95+
}
96+
}
97+
}
98+
}
99+
100+
fn future_trait_ref<'tcx>(cx: &LateContext<'_, 'tcx>, ty: &'tcx Ty<'tcx>) -> Option<&'tcx TraitRef<'tcx>> {
101+
if_chain! {
102+
if let TyKind::Def(item_id, _) = ty.kind;
103+
let item = cx.tcx.hir().item(item_id.id);
104+
if let ItemKind::OpaqueTy(opaque) = &item.kind;
105+
if opaque.bounds.len() == 1;
106+
if let GenericBound::Trait(poly, _) = &opaque.bounds[0];
107+
if poly.trait_ref.trait_def_id() == cx.tcx.lang_items().future_trait();
108+
then {
109+
return Some(&poly.trait_ref);
110+
}
111+
}
112+
113+
None
114+
}
115+
116+
fn future_output_ty<'tcx>(trait_ref: &'tcx TraitRef<'tcx>) -> Option<&'tcx Ty<'tcx>> {
117+
if_chain! {
118+
if let Some(segment) = trait_ref.path.segments.last();
119+
if let Some(args) = segment.args;
120+
if args.bindings.len() == 1;
121+
let binding = &args.bindings[0];
122+
if binding.ident.as_str() == "Output";
123+
if let TypeBindingKind::Equality{ty: output} = binding.kind;
124+
then {
125+
return Some(output)
126+
}
127+
}
128+
129+
None
130+
}
131+
132+
fn desugared_async_block<'tcx>(cx: &LateContext<'_, 'tcx>, block: &'tcx Block<'tcx>) -> Option<&'tcx Body<'tcx>> {
133+
if_chain! {
134+
if let Some(block_expr) = block.expr;
135+
if let Some(args) = match_function_call(cx, block_expr, &FUTURE_FROM_GENERATOR);
136+
if args.len() == 1;
137+
if let Expr{kind: ExprKind::Closure(_, _, body_id, ..), ..} = args[0];
138+
let closure_body = cx.tcx.hir().body(body_id);
139+
if let Some(GeneratorKind::Async(AsyncGeneratorKind::Block)) = closure_body.generator_kind;
140+
then {
141+
return Some(closure_body);
142+
}
143+
}
144+
145+
None
146+
}
147+
148+
fn suggested_ret(cx: &LateContext<'_, '_>, output: &Ty<'_>) -> Option<(&'static str, String)> {
149+
match output.kind {
150+
TyKind::Tup(tys) if tys.is_empty() => {
151+
let sugg = "remove the return type";
152+
Some((sugg, "".into()))
153+
},
154+
_ => {
155+
let sugg = "return the output of the future directly";
156+
snippet_opt(cx, output.span).map(|snip| (sugg, format!("-> {}", snip)))
157+
},
158+
}
159+
}

clippy_lints/src/utils/paths.rs

+1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ pub const FMT_ARGUMENTS_NEW_V1_FORMATTED: [&str; 4] = ["core", "fmt", "Arguments
4242
pub const FMT_ARGUMENTV1_NEW: [&str; 4] = ["core", "fmt", "ArgumentV1", "new"];
4343
pub const FROM_FROM: [&str; 4] = ["core", "convert", "From", "from"];
4444
pub const FROM_TRAIT: [&str; 3] = ["core", "convert", "From"];
45+
pub const FUTURE_FROM_GENERATOR: [&str; 3] = ["core", "future", "from_generator"];
4546
pub const HASH: [&str; 2] = ["hash", "Hash"];
4647
pub const HASHMAP: [&str; 5] = ["std", "collections", "hash", "map", "HashMap"];
4748
pub const HASHMAP_ENTRY: [&str; 5] = ["std", "collections", "hash", "map", "Entry"];

src/lintlist/mod.rs

+7
Original file line numberDiff line numberDiff line change
@@ -1081,6 +1081,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
10811081
deprecation: None,
10821082
module: "main_recursion",
10831083
},
1084+
Lint {
1085+
name: "manual_async_fn",
1086+
group: "style",
1087+
desc: "manual implementations of `async` functions can be simplified using the dedicated syntax",
1088+
deprecation: None,
1089+
module: "manual_async_fn",
1090+
},
10841091
Lint {
10851092
name: "manual_memcpy",
10861093
group: "perf",

tests/ui/future_not_send.rs

+1
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ impl Dummy {
4141
self.private_future().await;
4242
}
4343

44+
#[allow(clippy::manual_async_fn)]
4445
pub fn public_send(&self) -> impl std::future::Future<Output = bool> {
4546
async { false }
4647
}

tests/ui/future_not_send.stderr

+3-3
Original file line numberDiff line numberDiff line change
@@ -96,13 +96,13 @@ LL | }
9696
= note: `std::rc::Rc<[u8]>` doesn't implement `std::marker::Sync`
9797

9898
error: future cannot be sent between threads safely
99-
--> $DIR/future_not_send.rs:49:37
99+
--> $DIR/future_not_send.rs:50:37
100100
|
101101
LL | async fn generic_future<T>(t: T) -> T
102102
| ^ future returned by `generic_future` is not `Send`
103103
|
104104
note: future is not `Send` as this value is used across an await
105-
--> $DIR/future_not_send.rs:54:5
105+
--> $DIR/future_not_send.rs:55:5
106106
|
107107
LL | let rt = &t;
108108
| -- has type `&T` which is not `Send`
@@ -114,7 +114,7 @@ LL | }
114114
= note: `T` doesn't implement `std::marker::Sync`
115115

116116
error: future cannot be sent between threads safely
117-
--> $DIR/future_not_send.rs:65:34
117+
--> $DIR/future_not_send.rs:66:34
118118
|
119119
LL | async fn unclear_future<T>(t: T) {}
120120
| ^

tests/ui/manual_async_fn.fixed

+67
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// run-rustfix
2+
// edition:2018
3+
#![warn(clippy::manual_async_fn)]
4+
#![allow(unused)]
5+
6+
use std::future::Future;
7+
8+
async fn fut() -> i32 { 42 }
9+
10+
async fn empty_fut() {}
11+
12+
async fn core_fut() -> i32 { 42 }
13+
14+
// should be ignored
15+
fn has_other_stmts() -> impl core::future::Future<Output = i32> {
16+
let _ = 42;
17+
async move { 42 }
18+
}
19+
20+
// should be ignored
21+
fn not_fut() -> i32 {
22+
42
23+
}
24+
25+
// should be ignored
26+
async fn already_async() -> impl Future<Output = i32> {
27+
async { 42 }
28+
}
29+
30+
struct S {}
31+
impl S {
32+
async fn inh_fut() -> i32 {
33+
// NOTE: this code is here just to check that the identation is correct in the suggested fix
34+
let a = 42;
35+
let b = 21;
36+
if a < b {
37+
let c = 21;
38+
let d = 42;
39+
if c < d {
40+
let _ = 42;
41+
}
42+
}
43+
42
44+
}
45+
46+
async fn meth_fut(&self) -> i32 { 42 }
47+
48+
async fn empty_fut(&self) {}
49+
50+
// should be ignored
51+
fn not_fut(&self) -> i32 {
52+
42
53+
}
54+
55+
// should be ignored
56+
fn has_other_stmts() -> impl core::future::Future<Output = i32> {
57+
let _ = 42;
58+
async move { 42 }
59+
}
60+
61+
// should be ignored
62+
async fn already_async(&self) -> impl Future<Output = i32> {
63+
async { 42 }
64+
}
65+
}
66+
67+
fn main() {}

0 commit comments

Comments
 (0)