Skip to content

Commit

Permalink
[no_effect]: suggest adding return if applicable
Browse files Browse the repository at this point in the history
  • Loading branch information
Centri3 committed Jun 13, 2023
1 parent 8a1f0cd commit fcf15be
Show file tree
Hide file tree
Showing 3 changed files with 215 additions and 3 deletions.
47 changes: 44 additions & 3 deletions clippy_lints/src/no_effect.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
use clippy_utils::diagnostics::{span_lint_hir, span_lint_hir_and_then};
use clippy_utils::is_lint_allowed;
use clippy_utils::peel_blocks;
use clippy_utils::source::snippet_opt;
use clippy_utils::ty::has_drop;
use clippy_utils::{get_parent_node, is_lint_allowed};
use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{is_range_literal, BinOpKind, BlockCheckMode, Expr, ExprKind, PatKind, Stmt, StmtKind, UnsafeSource};
use rustc_hir::{
is_range_literal, BinOpKind, BlockCheckMode, Expr, ExprKind, FnRetTy, ItemKind, Node, PatKind, Stmt, StmtKind,
UnsafeSource,
};
use rustc_hir_analysis::hir_ty_to_ty;
use rustc_infer::infer::TyCtxtInferExt as _;
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint};
Expand Down Expand Up @@ -86,7 +91,43 @@ impl<'tcx> LateLintPass<'tcx> for NoEffect {
fn check_no_effect(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool {
if let StmtKind::Semi(expr) = stmt.kind {
if has_no_effect(cx, expr) {
span_lint_hir(cx, NO_EFFECT, expr.hir_id, stmt.span, "statement with no effect");
span_lint_hir_and_then(
cx,
NO_EFFECT,
expr.hir_id,
stmt.span,
"statement with no effect",
|diag| {
for parent in cx.tcx.hir().parent_iter(stmt.hir_id) {
if let Node::Item(item) = parent.1
&& let ItemKind::Fn(sig, ..) = item.kind
&& let FnRetTy::Return(ret_ty) = sig.decl.output
&& let Some(Node::Block(block)) = get_parent_node(cx.tcx, stmt.hir_id)
&& let [.., final_stmt] = block.stmts
&& final_stmt.hir_id == stmt.hir_id
{
let expr_ty = cx.typeck_results().expr_ty(expr);
let mut ret_ty = hir_ty_to_ty(cx.tcx, ret_ty);

// Remove `impl Future<Output = T>` to get `T`
if cx.tcx.ty_is_opaque_future(ret_ty) &&
let Some(true_ret_ty) = cx.tcx.infer_ctxt().build().get_impl_future_output_ty(ret_ty)
{
ret_ty = true_ret_ty;
}

if ret_ty == expr_ty {
diag.span_suggestion(
stmt.span.shrink_to_lo(),
"did you mean to return it?",
"return ",
Applicability::MaybeIncorrect,
);
}
}
}
},
);
return true;
}
} else if let StmtKind::Local(local) = stmt.kind {
Expand Down
81 changes: 81 additions & 0 deletions tests/ui/no_effect_return.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
#![allow(dead_code, unused)]
#![no_main]

use std::ops::ControlFlow;

fn a() -> u32 {
{
0u32;
}
0
}

async fn b() -> u32 {
{
0u32;
}
0
}

type C = i32;
async fn c() -> C {
{
0i32 as C;
}
0
}

fn d() -> u128 {
{
// not last stmt
0u128;
println!("lol");
}
0
}

fn e() -> u32 {
{
// mismatched types
0u16;
}
0
}

fn f() -> [u16; 1] {
{
[1u16];
}
[1]
}

fn g() -> ControlFlow<()> {
{
ControlFlow::Break::<()>(());
}
ControlFlow::Continue(())
}

fn h() -> Vec<u16> {
{
// function call, so this won't trigger `no_effect`. not an issue with this change, but the
// lint itself (but also not really.)
vec![0u16];
}
vec![]
}

fn i() -> () {
{
();
}
()
}

fn j() {
{
// does not suggest on function without explicit return type
();
}
()
}
90 changes: 90 additions & 0 deletions tests/ui/no_effect_return.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
error: unneeded unit return type
--> $DIR/no_effect_return.rs:68:7
|
LL | fn i() -> () {
| ^^^^^^ help: remove the `-> ()`
|
= note: `-D clippy::unused-unit` implied by `-D warnings`

error: unneeded unit expression
--> $DIR/no_effect_return.rs:72:5
|
LL | ()
| ^^ help: remove the final `()`

error: unneeded unit expression
--> $DIR/no_effect_return.rs:80:5
|
LL | ()
| ^^ help: remove the final `()`

error: statement with no effect
--> $DIR/no_effect_return.rs:8:9
|
LL | 0u32;
| -^^^^
| |
| help: did you mean to return it?: `return`
|
= note: `-D clippy::no-effect` implied by `-D warnings`

error: statement with no effect
--> $DIR/no_effect_return.rs:15:9
|
LL | 0u32;
| -^^^^
| |
| help: did you mean to return it?: `return`

error: statement with no effect
--> $DIR/no_effect_return.rs:23:9
|
LL | 0i32 as C;
| -^^^^^^^^^
| |
| help: did you mean to return it?: `return`

error: statement with no effect
--> $DIR/no_effect_return.rs:31:9
|
LL | 0u128;
| ^^^^^^

error: statement with no effect
--> $DIR/no_effect_return.rs:40:9
|
LL | 0u16;
| ^^^^^

error: statement with no effect
--> $DIR/no_effect_return.rs:47:9
|
LL | [1u16];
| -^^^^^^
| |
| help: did you mean to return it?: `return`

error: statement with no effect
--> $DIR/no_effect_return.rs:54:9
|
LL | ControlFlow::Break::<()>(());
| -^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| help: did you mean to return it?: `return`

error: statement with no effect
--> $DIR/no_effect_return.rs:70:9
|
LL | ();
| -^^
| |
| help: did you mean to return it?: `return`

error: statement with no effect
--> $DIR/no_effect_return.rs:78:9
|
LL | ();
| ^^^

error: aborting due to 12 previous errors

0 comments on commit fcf15be

Please sign in to comment.