Skip to content

Commit b2c8c02

Browse files
authored
Fix unused_async FP on function with todo! (rust-lang#15308)
Closes rust-lang/rust-clippy#15305 changelog: [`unused_async`] fix FP on function with `todo!`
2 parents d4dea4b + 6a87804 commit b2c8c02

File tree

4 files changed

+78
-23
lines changed

4 files changed

+78
-23
lines changed

clippy_lints/src/unused_async.rs

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
use clippy_utils::diagnostics::span_lint_hir_and_then;
22
use clippy_utils::is_def_id_trait_method;
3+
use clippy_utils::usage::is_todo_unimplemented_stub;
34
use rustc_hir::def::DefKind;
45
use rustc_hir::intravisit::{FnKind, Visitor, walk_expr, walk_fn};
5-
use rustc_hir::{Body, Defaultness, Expr, ExprKind, FnDecl, HirId, Node, TraitItem, YieldSource};
6+
use rustc_hir::{
7+
Body, Closure, ClosureKind, CoroutineDesugaring, CoroutineKind, Defaultness, Expr, ExprKind, FnDecl, HirId, Node,
8+
TraitItem, YieldSource,
9+
};
610
use rustc_lint::{LateContext, LateLintPass};
711
use rustc_middle::hir::nested_filter;
812
use rustc_session::impl_lint_pass;
@@ -81,11 +85,8 @@ impl<'tcx> Visitor<'tcx> for AsyncFnVisitor<'_, 'tcx> {
8185

8286
let is_async_block = matches!(
8387
ex.kind,
84-
ExprKind::Closure(rustc_hir::Closure {
85-
kind: rustc_hir::ClosureKind::Coroutine(rustc_hir::CoroutineKind::Desugared(
86-
rustc_hir::CoroutineDesugaring::Async,
87-
_
88-
)),
88+
ExprKind::Closure(Closure {
89+
kind: ClosureKind::Coroutine(CoroutineKind::Desugared(CoroutineDesugaring::Async, _)),
8990
..
9091
})
9192
);
@@ -120,6 +121,7 @@ impl<'tcx> LateLintPass<'tcx> for UnusedAsync {
120121
&& fn_kind.asyncness().is_async()
121122
&& !is_def_id_trait_method(cx, def_id)
122123
&& !is_default_trait_impl(cx, def_id)
124+
&& !async_fn_contains_todo_unimplemented_macro(cx, body)
123125
{
124126
let mut visitor = AsyncFnVisitor {
125127
cx,
@@ -203,3 +205,18 @@ fn is_default_trait_impl(cx: &LateContext<'_>, def_id: LocalDefId) -> bool {
203205
})
204206
)
205207
}
208+
209+
fn async_fn_contains_todo_unimplemented_macro(cx: &LateContext<'_>, body: &Body<'_>) -> bool {
210+
if let ExprKind::Closure(closure) = body.value.kind
211+
&& let ClosureKind::Coroutine(CoroutineKind::Desugared(CoroutineDesugaring::Async, _)) = closure.kind
212+
&& let body = cx.tcx.hir_body(closure.body)
213+
&& let ExprKind::Block(block, _) = body.value.kind
214+
&& block.stmts.is_empty()
215+
&& let Some(expr) = block.expr
216+
&& let ExprKind::DropTemps(inner) = expr.kind
217+
{
218+
return is_todo_unimplemented_stub(cx, inner);
219+
}
220+
221+
false
222+
}

clippy_lints/src/unused_self.rs

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
use clippy_config::Conf;
22
use clippy_utils::diagnostics::span_lint_and_help;
3-
use clippy_utils::macros::root_macro_call_first_node;
4-
use clippy_utils::sym;
3+
use clippy_utils::usage::is_todo_unimplemented_stub;
54
use clippy_utils::visitors::is_local_used;
6-
use rustc_hir::{Body, Impl, ImplItem, ImplItemKind, ItemKind};
5+
use rustc_hir::{Impl, ImplItem, ImplItemKind, ItemKind};
76
use rustc_lint::{LateContext, LateLintPass};
87
use rustc_session::impl_lint_pass;
9-
use std::ops::ControlFlow;
108

119
declare_clippy_lint! {
1210
/// ### What it does
@@ -60,26 +58,14 @@ impl<'tcx> LateLintPass<'tcx> for UnusedSelf {
6058
let parent = cx.tcx.hir_get_parent_item(impl_item.hir_id()).def_id;
6159
let parent_item = cx.tcx.hir_expect_item(parent);
6260
let assoc_item = cx.tcx.associated_item(impl_item.owner_id);
63-
let contains_todo = |cx, body: &'_ Body<'_>| -> bool {
64-
clippy_utils::visitors::for_each_expr_without_closures(body.value, |e| {
65-
if let Some(macro_call) = root_macro_call_first_node(cx, e)
66-
&& cx.tcx.is_diagnostic_item(sym::todo_macro, macro_call.def_id)
67-
{
68-
ControlFlow::Break(())
69-
} else {
70-
ControlFlow::Continue(())
71-
}
72-
})
73-
.is_some()
74-
};
7561
if let ItemKind::Impl(Impl { of_trait: None, .. }) = parent_item.kind
7662
&& assoc_item.is_method()
7763
&& let ImplItemKind::Fn(.., body_id) = &impl_item.kind
7864
&& (!cx.effective_visibilities.is_exported(impl_item.owner_id.def_id) || !self.avoid_breaking_exported_api)
7965
&& let body = cx.tcx.hir_body(*body_id)
8066
&& let [self_param, ..] = body.params
8167
&& !is_local_used(cx, body, self_param.pat.hir_id)
82-
&& !contains_todo(cx, body)
68+
&& !is_todo_unimplemented_stub(cx, body.value)
8369
{
8470
span_lint_and_help(
8571
cx,

clippy_utils/src/usage.rs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use crate::macros::root_macro_call_first_node;
12
use crate::visitors::{Descend, Visitable, for_each_expr, for_each_expr_without_closures};
23
use crate::{self as utils, get_enclosing_loop_or_multi_call_closure};
34
use core::ops::ControlFlow;
@@ -9,6 +10,7 @@ use rustc_lint::LateContext;
910
use rustc_middle::hir::nested_filter;
1011
use rustc_middle::mir::FakeReadCause;
1112
use rustc_middle::ty;
13+
use rustc_span::sym;
1214

1315
/// Returns a set of mutated local variable IDs, or `None` if mutations could not be determined.
1416
pub fn mutated_variables<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) -> Option<HirIdSet> {
@@ -140,6 +142,46 @@ impl<'tcx> Visitor<'tcx> for BindingUsageFinder<'_, 'tcx> {
140142
}
141143
}
142144

145+
/// Checks if the given expression is a macro call to `todo!()` or `unimplemented!()`.
146+
pub fn is_todo_unimplemented_macro(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
147+
root_macro_call_first_node(cx, expr).is_some_and(|macro_call| {
148+
[sym::todo_macro, sym::unimplemented_macro]
149+
.iter()
150+
.any(|&sym| cx.tcx.is_diagnostic_item(sym, macro_call.def_id))
151+
})
152+
}
153+
154+
/// Checks if the given expression is a stub, i.e., a `todo!()` or `unimplemented!()` expression,
155+
/// or a block whose last expression is a `todo!()` or `unimplemented!()`.
156+
pub fn is_todo_unimplemented_stub(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
157+
if let ExprKind::Block(block, _) = expr.kind {
158+
if let Some(last_expr) = block.expr {
159+
return is_todo_unimplemented_macro(cx, last_expr);
160+
}
161+
162+
return block.stmts.last().is_some_and(|stmt| {
163+
if let hir::StmtKind::Expr(expr) | hir::StmtKind::Semi(expr) = stmt.kind {
164+
return is_todo_unimplemented_macro(cx, expr);
165+
}
166+
false
167+
});
168+
}
169+
170+
is_todo_unimplemented_macro(cx, expr)
171+
}
172+
173+
/// Checks if the given expression contains macro call to `todo!()` or `unimplemented!()`.
174+
pub fn contains_todo_unimplement_macro(cx: &LateContext<'_>, expr: &'_ Expr<'_>) -> bool {
175+
for_each_expr_without_closures(expr, |e| {
176+
if is_todo_unimplemented_macro(cx, e) {
177+
ControlFlow::Break(())
178+
} else {
179+
ControlFlow::Continue(())
180+
}
181+
})
182+
.is_some()
183+
}
184+
143185
pub fn contains_return_break_continue_macro(expression: &Expr<'_>) -> bool {
144186
for_each_expr_without_closures(expression, |e| {
145187
match e.kind {

tests/ui/unused_async.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,3 +127,13 @@ mod issue14704 {
127127
async fn cancel(self: Arc<Self>) {}
128128
}
129129
}
130+
131+
mod issue15305 {
132+
async fn todo_task() -> Result<(), String> {
133+
todo!("Implement task");
134+
}
135+
136+
async fn unimplemented_task() -> Result<(), String> {
137+
unimplemented!("Implement task");
138+
}
139+
}

0 commit comments

Comments
 (0)