Skip to content

Commit fcf15be

Browse files
committed
[no_effect]: suggest adding return if applicable
1 parent 8a1f0cd commit fcf15be

File tree

3 files changed

+215
-3
lines changed

3 files changed

+215
-3
lines changed

clippy_lints/src/no_effect.rs

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
use clippy_utils::diagnostics::{span_lint_hir, span_lint_hir_and_then};
2-
use clippy_utils::is_lint_allowed;
32
use clippy_utils::peel_blocks;
43
use clippy_utils::source::snippet_opt;
54
use clippy_utils::ty::has_drop;
5+
use clippy_utils::{get_parent_node, is_lint_allowed};
66
use rustc_errors::Applicability;
77
use rustc_hir::def::{DefKind, Res};
8-
use rustc_hir::{is_range_literal, BinOpKind, BlockCheckMode, Expr, ExprKind, PatKind, Stmt, StmtKind, UnsafeSource};
8+
use rustc_hir::{
9+
is_range_literal, BinOpKind, BlockCheckMode, Expr, ExprKind, FnRetTy, ItemKind, Node, PatKind, Stmt, StmtKind,
10+
UnsafeSource,
11+
};
12+
use rustc_hir_analysis::hir_ty_to_ty;
13+
use rustc_infer::infer::TyCtxtInferExt as _;
914
use rustc_lint::{LateContext, LateLintPass, LintContext};
1015
use rustc_middle::lint::in_external_macro;
1116
use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -86,7 +91,43 @@ impl<'tcx> LateLintPass<'tcx> for NoEffect {
8691
fn check_no_effect(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool {
8792
if let StmtKind::Semi(expr) = stmt.kind {
8893
if has_no_effect(cx, expr) {
89-
span_lint_hir(cx, NO_EFFECT, expr.hir_id, stmt.span, "statement with no effect");
94+
span_lint_hir_and_then(
95+
cx,
96+
NO_EFFECT,
97+
expr.hir_id,
98+
stmt.span,
99+
"statement with no effect",
100+
|diag| {
101+
for parent in cx.tcx.hir().parent_iter(stmt.hir_id) {
102+
if let Node::Item(item) = parent.1
103+
&& let ItemKind::Fn(sig, ..) = item.kind
104+
&& let FnRetTy::Return(ret_ty) = sig.decl.output
105+
&& let Some(Node::Block(block)) = get_parent_node(cx.tcx, stmt.hir_id)
106+
&& let [.., final_stmt] = block.stmts
107+
&& final_stmt.hir_id == stmt.hir_id
108+
{
109+
let expr_ty = cx.typeck_results().expr_ty(expr);
110+
let mut ret_ty = hir_ty_to_ty(cx.tcx, ret_ty);
111+
112+
// Remove `impl Future<Output = T>` to get `T`
113+
if cx.tcx.ty_is_opaque_future(ret_ty) &&
114+
let Some(true_ret_ty) = cx.tcx.infer_ctxt().build().get_impl_future_output_ty(ret_ty)
115+
{
116+
ret_ty = true_ret_ty;
117+
}
118+
119+
if ret_ty == expr_ty {
120+
diag.span_suggestion(
121+
stmt.span.shrink_to_lo(),
122+
"did you mean to return it?",
123+
"return ",
124+
Applicability::MaybeIncorrect,
125+
);
126+
}
127+
}
128+
}
129+
},
130+
);
90131
return true;
91132
}
92133
} else if let StmtKind::Local(local) = stmt.kind {

tests/ui/no_effect_return.rs

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
#![allow(dead_code, unused)]
2+
#![no_main]
3+
4+
use std::ops::ControlFlow;
5+
6+
fn a() -> u32 {
7+
{
8+
0u32;
9+
}
10+
0
11+
}
12+
13+
async fn b() -> u32 {
14+
{
15+
0u32;
16+
}
17+
0
18+
}
19+
20+
type C = i32;
21+
async fn c() -> C {
22+
{
23+
0i32 as C;
24+
}
25+
0
26+
}
27+
28+
fn d() -> u128 {
29+
{
30+
// not last stmt
31+
0u128;
32+
println!("lol");
33+
}
34+
0
35+
}
36+
37+
fn e() -> u32 {
38+
{
39+
// mismatched types
40+
0u16;
41+
}
42+
0
43+
}
44+
45+
fn f() -> [u16; 1] {
46+
{
47+
[1u16];
48+
}
49+
[1]
50+
}
51+
52+
fn g() -> ControlFlow<()> {
53+
{
54+
ControlFlow::Break::<()>(());
55+
}
56+
ControlFlow::Continue(())
57+
}
58+
59+
fn h() -> Vec<u16> {
60+
{
61+
// function call, so this won't trigger `no_effect`. not an issue with this change, but the
62+
// lint itself (but also not really.)
63+
vec![0u16];
64+
}
65+
vec![]
66+
}
67+
68+
fn i() -> () {
69+
{
70+
();
71+
}
72+
()
73+
}
74+
75+
fn j() {
76+
{
77+
// does not suggest on function without explicit return type
78+
();
79+
}
80+
()
81+
}

tests/ui/no_effect_return.stderr

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
error: unneeded unit return type
2+
--> $DIR/no_effect_return.rs:68:7
3+
|
4+
LL | fn i() -> () {
5+
| ^^^^^^ help: remove the `-> ()`
6+
|
7+
= note: `-D clippy::unused-unit` implied by `-D warnings`
8+
9+
error: unneeded unit expression
10+
--> $DIR/no_effect_return.rs:72:5
11+
|
12+
LL | ()
13+
| ^^ help: remove the final `()`
14+
15+
error: unneeded unit expression
16+
--> $DIR/no_effect_return.rs:80:5
17+
|
18+
LL | ()
19+
| ^^ help: remove the final `()`
20+
21+
error: statement with no effect
22+
--> $DIR/no_effect_return.rs:8:9
23+
|
24+
LL | 0u32;
25+
| -^^^^
26+
| |
27+
| help: did you mean to return it?: `return`
28+
|
29+
= note: `-D clippy::no-effect` implied by `-D warnings`
30+
31+
error: statement with no effect
32+
--> $DIR/no_effect_return.rs:15:9
33+
|
34+
LL | 0u32;
35+
| -^^^^
36+
| |
37+
| help: did you mean to return it?: `return`
38+
39+
error: statement with no effect
40+
--> $DIR/no_effect_return.rs:23:9
41+
|
42+
LL | 0i32 as C;
43+
| -^^^^^^^^^
44+
| |
45+
| help: did you mean to return it?: `return`
46+
47+
error: statement with no effect
48+
--> $DIR/no_effect_return.rs:31:9
49+
|
50+
LL | 0u128;
51+
| ^^^^^^
52+
53+
error: statement with no effect
54+
--> $DIR/no_effect_return.rs:40:9
55+
|
56+
LL | 0u16;
57+
| ^^^^^
58+
59+
error: statement with no effect
60+
--> $DIR/no_effect_return.rs:47:9
61+
|
62+
LL | [1u16];
63+
| -^^^^^^
64+
| |
65+
| help: did you mean to return it?: `return`
66+
67+
error: statement with no effect
68+
--> $DIR/no_effect_return.rs:54:9
69+
|
70+
LL | ControlFlow::Break::<()>(());
71+
| -^^^^^^^^^^^^^^^^^^^^^^^^^^^^
72+
| |
73+
| help: did you mean to return it?: `return`
74+
75+
error: statement with no effect
76+
--> $DIR/no_effect_return.rs:70:9
77+
|
78+
LL | ();
79+
| -^^
80+
| |
81+
| help: did you mean to return it?: `return`
82+
83+
error: statement with no effect
84+
--> $DIR/no_effect_return.rs:78:9
85+
|
86+
LL | ();
87+
| ^^^
88+
89+
error: aborting due to 12 previous errors
90+

0 commit comments

Comments
 (0)