Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2025-02-17 14:38:46 +00:00
Code Issues Projects Releases Packages Wiki Activity

Auto merge of #5903 - jrqc:needless_return, r=ebroto,flip1995

Browse source 
Needless return

Fixes #5858
changelog: fix false positive [`needless_return`]
This commit is contained in:
bors 2020-08-16 17:08:45 +00:00
commit dff7e74b27
8 changed files with 422 additions and 353 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

124
clippy_lints/src/let_and_return.rs
View file

@ -1,124 +0,0 @@
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use rustc_hir::{Block, Expr, ExprKind, PatKind, StmtKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::hir::map::Map;
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::subst::GenericArgKind;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use crate::utils::{fn_def_id, in_macro, match_qpath, snippet_opt, span_lint_and_then};
declare_clippy_lint! {
/// **What it does:** Checks for `let`-bindings, which are subsequently
/// returned.
///
/// **Why is this bad?** It is just extraneous code. Remove it to make your code
/// more rusty.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// fn foo() -> String {
/// let x = String::new();
/// x
/// }
/// ```
/// instead, use
/// ```
/// fn foo() -> String {
/// String::new()
/// }
/// ```
pub LET_AND_RETURN,
style,
"creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block"
}
declare_lint_pass!(LetReturn => [LET_AND_RETURN]);
impl<'tcx> LateLintPass<'tcx> for LetReturn {
fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'_>) {
// we need both a let-binding stmt and an expr
if_chain! {
if let Some(retexpr) = block.expr;
if let Some(stmt) = block.stmts.iter().last();
if let StmtKind::Local(local) = &stmt.kind;
if local.ty.is_none();
if local.attrs.is_empty();
if let Some(initexpr) = &local.init;
if let PatKind::Binding(.., ident, _) = local.pat.kind;
if let ExprKind::Path(qpath) = &retexpr.kind;
if match_qpath(qpath, &[&*ident.name.as_str()]);
if !last_statement_borrows(cx, initexpr);
if !in_external_macro(cx.sess(), initexpr.span);
if !in_external_macro(cx.sess(), retexpr.span);
if !in_external_macro(cx.sess(), local.span);
if !in_macro(local.span);
then {
span_lint_and_then(
cx,
LET_AND_RETURN,
retexpr.span,
"returning the result of a `let` binding from a block",
|err| {
err.span_label(local.span, "unnecessary `let` binding");
if let Some(snippet) = snippet_opt(cx, initexpr.span) {
err.multipart_suggestion(
"return the expression directly",
vec![
(local.span, String::new()),
(retexpr.span, snippet),
],
Applicability::MachineApplicable,
);
} else {
err.span_help(initexpr.span, "this expression can be directly returned");
}
},
);
}
}
}
}
fn last_statement_borrows<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
let mut visitor = BorrowVisitor { cx, borrows: false };
walk_expr(&mut visitor, expr);
visitor.borrows
}
struct BorrowVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
borrows: bool,
}
impl<'tcx> Visitor<'tcx> for BorrowVisitor<'_, 'tcx> {
type Map = Map<'tcx>;
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if self.borrows {
return;
}
if let Some(def_id) = fn_def_id(self.cx, expr) {
self.borrows = self
.cx
.tcx
.fn_sig(def_id)
.output()
.skip_binder()
.walk()
.any(|arg| matches!(arg.unpack(), GenericArgKind::Lifetime(_)));
}
walk_expr(self, expr);
}
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
}

18
clippy_lints/src/lib.rs
View file

@ -218,7 +218,6 @@ mod large_const_arrays;
mod large_enum_variant; mod large_enum_variant;
mod large_stack_arrays; mod large_stack_arrays;
mod len_zero; mod len_zero;
mod let_and_return;
mod let_if_seq; mod let_if_seq;
mod let_underscore; mod let_underscore;
mod lifetimes; mod lifetimes;
@ -311,6 +310,7 @@ mod unnested_or_patterns;
mod unsafe_removed_from_name; mod unsafe_removed_from_name;
mod unused_io_amount; mod unused_io_amount;
mod unused_self; mod unused_self;
mod unused_unit;
mod unwrap; mod unwrap;
mod use_self; mod use_self;
mod useless_conversion; mod useless_conversion;
@ -587,7 +587,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&large_stack_arrays::LARGE_STACK_ARRAYS, &large_stack_arrays::LARGE_STACK_ARRAYS,
&len_zero::LEN_WITHOUT_IS_EMPTY, &len_zero::LEN_WITHOUT_IS_EMPTY,
&len_zero::LEN_ZERO, &len_zero::LEN_ZERO,
&let_and_return::LET_AND_RETURN,
&let_if_seq::USELESS_LET_IF_SEQ, &let_if_seq::USELESS_LET_IF_SEQ,
&let_underscore::LET_UNDERSCORE_LOCK, &let_underscore::LET_UNDERSCORE_LOCK,
&let_underscore::LET_UNDERSCORE_MUST_USE, &let_underscore::LET_UNDERSCORE_MUST_USE,
@ -771,8 +770,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&regex::INVALID_REGEX, &regex::INVALID_REGEX,
&regex::TRIVIAL_REGEX, &regex::TRIVIAL_REGEX,
&repeat_once::REPEAT_ONCE, &repeat_once::REPEAT_ONCE,
&returns::LET_AND_RETURN,
&returns::NEEDLESS_RETURN, &returns::NEEDLESS_RETURN,
&returns::UNUSED_UNIT,
&serde_api::SERDE_API_MISUSE, &serde_api::SERDE_API_MISUSE,
&shadow::SHADOW_REUSE, &shadow::SHADOW_REUSE,
&shadow::SHADOW_SAME, &shadow::SHADOW_SAME,
@ -843,6 +842,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME, &unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME,
&unused_io_amount::UNUSED_IO_AMOUNT, &unused_io_amount::UNUSED_IO_AMOUNT,
&unused_self::UNUSED_SELF, &unused_self::UNUSED_SELF,
&unused_unit::UNUSED_UNIT,
&unwrap::PANICKING_UNWRAP, &unwrap::PANICKING_UNWRAP,
&unwrap::UNNECESSARY_UNWRAP, &unwrap::UNNECESSARY_UNWRAP,
&use_self::USE_SELF, &use_self::USE_SELF,
@ -1029,8 +1029,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_early_pass(|| box misc_early::MiscEarlyLints); store.register_early_pass(|| box misc_early::MiscEarlyLints);
store.register_early_pass(|| box redundant_closure_call::RedundantClosureCall); store.register_early_pass(|| box redundant_closure_call::RedundantClosureCall);
store.register_late_pass(|| box redundant_closure_call::RedundantClosureCall); store.register_late_pass(|| box redundant_closure_call::RedundantClosureCall);
store.register_early_pass(|| box returns::Return); store.register_early_pass(|| box unused_unit::UnusedUnit);
store.register_late_pass(|| box let_and_return::LetReturn); store.register_late_pass(|| box returns::Return);
store.register_early_pass(|| box collapsible_if::CollapsibleIf); store.register_early_pass(|| box collapsible_if::CollapsibleIf);
store.register_early_pass(|| box items_after_statements::ItemsAfterStatements); store.register_early_pass(|| box items_after_statements::ItemsAfterStatements);
store.register_early_pass(|| box precedence::Precedence); store.register_early_pass(|| box precedence::Precedence);
@ -1288,7 +1288,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&large_enum_variant::LARGE_ENUM_VARIANT), LintId::of(&large_enum_variant::LARGE_ENUM_VARIANT),
LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY), LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
LintId::of(&len_zero::LEN_ZERO), LintId::of(&len_zero::LEN_ZERO),
LintId::of(&let_and_return::LET_AND_RETURN),
LintId::of(&let_underscore::LET_UNDERSCORE_LOCK), LintId::of(&let_underscore::LET_UNDERSCORE_LOCK),
LintId::of(&lifetimes::EXTRA_UNUSED_LIFETIMES), LintId::of(&lifetimes::EXTRA_UNUSED_LIFETIMES),
LintId::of(&lifetimes::NEEDLESS_LIFETIMES), LintId::of(&lifetimes::NEEDLESS_LIFETIMES),
@ -1418,8 +1417,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&regex::INVALID_REGEX), LintId::of(&regex::INVALID_REGEX),
LintId::of(&regex::TRIVIAL_REGEX), LintId::of(&regex::TRIVIAL_REGEX),
LintId::of(&repeat_once::REPEAT_ONCE), LintId::of(&repeat_once::REPEAT_ONCE),
LintId::of(&returns::LET_AND_RETURN),
LintId::of(&returns::NEEDLESS_RETURN), LintId::of(&returns::NEEDLESS_RETURN),
LintId::of(&returns::UNUSED_UNIT),
LintId::of(&serde_api::SERDE_API_MISUSE), LintId::of(&serde_api::SERDE_API_MISUSE),
LintId::of(&single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS), LintId::of(&single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),
LintId::of(&slow_vector_initialization::SLOW_VECTOR_INITIALIZATION), LintId::of(&slow_vector_initialization::SLOW_VECTOR_INITIALIZATION),
@ -1466,6 +1465,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&unnecessary_sort_by::UNNECESSARY_SORT_BY), LintId::of(&unnecessary_sort_by::UNNECESSARY_SORT_BY),
LintId::of(&unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME), LintId::of(&unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME),
LintId::of(&unused_io_amount::UNUSED_IO_AMOUNT), LintId::of(&unused_io_amount::UNUSED_IO_AMOUNT),
LintId::of(&unused_unit::UNUSED_UNIT),
LintId::of(&unwrap::PANICKING_UNWRAP), LintId::of(&unwrap::PANICKING_UNWRAP),
LintId::of(&unwrap::UNNECESSARY_UNWRAP), LintId::of(&unwrap::UNNECESSARY_UNWRAP),
LintId::of(&useless_conversion::USELESS_CONVERSION), LintId::of(&useless_conversion::USELESS_CONVERSION),
@ -1506,7 +1506,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&inherent_to_string::INHERENT_TO_STRING), LintId::of(&inherent_to_string::INHERENT_TO_STRING),
LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY), LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
LintId::of(&len_zero::LEN_ZERO), LintId::of(&len_zero::LEN_ZERO),
LintId::of(&let_and_return::LET_AND_RETURN),
LintId::of(&literal_representation::INCONSISTENT_DIGIT_GROUPING), LintId::of(&literal_representation::INCONSISTENT_DIGIT_GROUPING),
LintId::of(&loops::EMPTY_LOOP), LintId::of(&loops::EMPTY_LOOP),
LintId::of(&loops::FOR_KV_MAP), LintId::of(&loops::FOR_KV_MAP),
@ -1561,8 +1560,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&redundant_field_names::REDUNDANT_FIELD_NAMES), LintId::of(&redundant_field_names::REDUNDANT_FIELD_NAMES),
LintId::of(&redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES), LintId::of(&redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES),
LintId::of(&regex::TRIVIAL_REGEX), LintId::of(&regex::TRIVIAL_REGEX),
LintId::of(&returns::LET_AND_RETURN),
LintId::of(&returns::NEEDLESS_RETURN), LintId::of(&returns::NEEDLESS_RETURN),
LintId::of(&returns::UNUSED_UNIT),
LintId::of(&single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS), LintId::of(&single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),
LintId::of(&strings::STRING_LIT_AS_BYTES), LintId::of(&strings::STRING_LIT_AS_BYTES),
LintId::of(&tabs_in_doc_comments::TABS_IN_DOC_COMMENTS), LintId::of(&tabs_in_doc_comments::TABS_IN_DOC_COMMENTS),
@ -1571,6 +1570,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&types::FN_TO_NUMERIC_CAST), LintId::of(&types::FN_TO_NUMERIC_CAST),
LintId::of(&types::FN_TO_NUMERIC_CAST_WITH_TRUNCATION), LintId::of(&types::FN_TO_NUMERIC_CAST_WITH_TRUNCATION),
LintId::of(&unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME), LintId::of(&unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME),
LintId::of(&unused_unit::UNUSED_UNIT),
LintId::of(&write::PRINTLN_EMPTY_STRING), LintId::of(&write::PRINTLN_EMPTY_STRING),
LintId::of(&write::PRINT_LITERAL), LintId::of(&write::PRINT_LITERAL),
LintId::of(&write::PRINT_WITH_NEWLINE), LintId::of(&write::PRINT_WITH_NEWLINE),

437
clippy_lints/src/returns.rs
View file

@ -1,14 +1,43 @@
use if_chain::if_chain; use if_chain::if_chain;
use rustc_ast::ast; use rustc_ast::ast::Attribute;
use rustc_ast::visit::FnKind;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; use rustc_hir::intravisit::{walk_expr, FnKind, NestedVisitorMap, Visitor};
use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl, HirId, MatchSource, PatKind, StmtKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::hir::map::Map;
use rustc_middle::lint::in_external_macro; use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::subst::GenericArgKind;
use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span; use rustc_span::source_map::Span;
use rustc_span::BytePos;
use crate::utils::{snippet_opt, span_lint_and_sugg, span_lint_and_then}; use crate::utils::{fn_def_id, in_macro, match_qpath, snippet_opt, span_lint_and_sugg, span_lint_and_then};
declare_clippy_lint! {
/// **What it does:** Checks for `let`-bindings, which are subsequently
/// returned.
///
/// **Why is this bad?** It is just extraneous code. Remove it to make your code
/// more rusty.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// fn foo() -> String {
/// let x = String::new();
/// x
/// }
/// ```
/// instead, use
/// ```
/// fn foo() -> String {
/// String::new()
/// }
/// ```
pub LET_AND_RETURN,
style,
"creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block"
}
declare_clippy_lint! { declare_clippy_lint! {
/// **What it does:** Checks for return statements at the end of a block. /// **What it does:** Checks for return statements at the end of a block.
@ -16,8 +45,7 @@ declare_clippy_lint! {
/// **Why is this bad?** Removing the `return` and semicolon will make the code /// **Why is this bad?** Removing the `return` and semicolon will make the code
/// more rusty. /// more rusty.
/// ///
/// **Known problems:** If the computation returning the value borrows a local /// **Known problems:** None.
/// variable, removing the `return` may run afoul of the borrow checker.
/// ///
/// **Example:** /// **Example:**
/// ```rust /// ```rust
@ -36,248 +64,223 @@ declare_clippy_lint! {
"using a return statement like `return expr;` where an expression would suffice" "using a return statement like `return expr;` where an expression would suffice"
} }
declare_clippy_lint! {
/// **What it does:** Checks for unit (`()`) expressions that can be removed.
///
/// **Why is this bad?** Such expressions add no value, but can make the code
/// less readable. Depending on formatting they can make a `break` or `return`
/// statement look like a function call.
///
/// **Known problems:** The lint currently misses unit return types in types,
/// e.g., the `F` in `fn generic_unit<F: Fn() -> ()>(f: F) { .. }`.
///
/// **Example:**
/// ```rust
/// fn return_unit() -> () {
/// ()
/// }
/// ```
pub UNUSED_UNIT,
style,
"needless unit expression"
}
#[derive(PartialEq, Eq, Copy, Clone)] #[derive(PartialEq, Eq, Copy, Clone)]
enum RetReplacement { enum RetReplacement {
Empty, Empty,
Block, Block,
} }
declare_lint_pass!(Return => [NEEDLESS_RETURN, UNUSED_UNIT]); declare_lint_pass!(Return => [LET_AND_RETURN, NEEDLESS_RETURN]);
impl Return { impl<'tcx> LateLintPass<'tcx> for Return {
// Check the final stmt or expr in a block for unnecessary return. fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'_>) {
fn check_block_return(&mut self, cx: &EarlyContext<'_>, block: &ast::Block) { // we need both a let-binding stmt and an expr
if let Some(stmt) = block.stmts.last() { if_chain! {
match stmt.kind { if let Some(retexpr) = block.expr;
ast::StmtKind::Expr(ref expr) | ast::StmtKind::Semi(ref expr) => { if let Some(stmt) = block.stmts.iter().last();
self.check_final_expr(cx, expr, Some(stmt.span), RetReplacement::Empty); if let StmtKind::Local(local) = &stmt.kind;
}, if local.ty.is_none();
_ => (), if local.attrs.is_empty();
if let Some(initexpr) = &local.init;
if let PatKind::Binding(.., ident, _) = local.pat.kind;
if let ExprKind::Path(qpath) = &retexpr.kind;
if match_qpath(qpath, &[&*ident.name.as_str()]);
if !last_statement_borrows(cx, initexpr);
if !in_external_macro(cx.sess(), initexpr.span);
if !in_external_macro(cx.sess(), retexpr.span);
if !in_external_macro(cx.sess(), local.span);
if !in_macro(local.span);
then {
span_lint_and_then(
cx,
LET_AND_RETURN,
retexpr.span,
"returning the result of a `let` binding from a block",
|err| {
err.span_label(local.span, "unnecessary `let` binding");
if let Some(snippet) = snippet_opt(cx, initexpr.span) {
err.multipart_suggestion(
"return the expression directly",
vec![
(local.span, String::new()),
(retexpr.span, snippet),
],
Applicability::MachineApplicable,
);
} else {
err.span_help(initexpr.span, "this expression can be directly returned");
}
},
);
} }
} }
} }
// Check the final expression in a block if it's a return. fn check_fn(
fn check_final_expr(
&mut self, &mut self,
cx: &EarlyContext<'_>, cx: &LateContext<'tcx>,
expr: &ast::Expr, kind: FnKind<'tcx>,
span: Option<Span>, _: &'tcx FnDecl<'tcx>,
replacement: RetReplacement, body: &'tcx Body<'tcx>,
_: Span,
_: HirId,
) { ) {
match expr.kind { match kind {
// simple return is always "bad" FnKind::Closure(_) => check_final_expr(cx, &body.value, Some(body.value.span), RetReplacement::Empty),
ast::ExprKind::Ret(ref inner) => { FnKind::ItemFn(..) | FnKind::Method(..) => {
// allow `#[cfg(a)] return a; #[cfg(b)] return b;` if let ExprKind::Block(ref block, _) = body.value.kind {
if !expr.attrs.iter().any(attr_is_cfg) { check_block_return(cx, block);
Self::emit_return_lint( }
},
}
}
}
fn attr_is_cfg(attr: &Attribute) -> bool {
attr.meta_item_list().is_some() && attr.has_name(sym!(cfg))
}
fn check_block_return<'tcx>(cx: &LateContext<'tcx>, block: &Block<'tcx>) {
if let Some(expr) = block.expr {
check_final_expr(cx, expr, Some(expr.span), RetReplacement::Empty);
} else if let Some(stmt) = block.stmts.iter().last() {
match stmt.kind {
StmtKind::Expr(ref expr) | StmtKind::Semi(ref expr) => {
check_final_expr(cx, expr, Some(stmt.span), RetReplacement::Empty);
},
_ => (),
}
}
}
fn check_final_expr<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'tcx>,
span: Option<Span>,
replacement: RetReplacement,
) {
match expr.kind {
// simple return is always "bad"
ExprKind::Ret(ref inner) => {
// allow `#[cfg(a)] return a; #[cfg(b)] return b;`
if !expr.attrs.iter().any(attr_is_cfg) {
let borrows = inner.map_or(false, |inner| last_statement_borrows(cx, inner));
if !borrows {
emit_return_lint(
cx, cx,
span.expect("`else return` is not possible"), span.expect("`else return` is not possible"),
inner.as_ref().map(|i| i.span), inner.as_ref().map(|i| i.span),
replacement, replacement,
); );
} }
}, }
// a whole block? check it! },
ast::ExprKind::Block(ref block, _) => { // a whole block? check it!
self.check_block_return(cx, block); ExprKind::Block(ref block, _) => {
}, check_block_return(cx, block);
// an if/if let expr, check both exprs },
// note, if without else is going to be a type checking error anyways // a match expr, check all arms
// (except for unit type functions) so we don't match it // an if/if let expr, check both exprs
ast::ExprKind::If(_, ref ifblock, Some(ref elsexpr)) => { // note, if without else is going to be a type checking error anyways
self.check_block_return(cx, ifblock); // (except for unit type functions) so we don't match it
self.check_final_expr(cx, elsexpr, None, RetReplacement::Empty); ExprKind::Match(_, ref arms, source) => match source {
}, MatchSource::Normal => {
// a match expr, check all arms for arm in arms.iter() {
ast::ExprKind::Match(_, ref arms) => { check_final_expr(cx, &arm.body, Some(arm.body.span), RetReplacement::Block);
for arm in arms {
self.check_final_expr(cx, &arm.body, Some(arm.body.span), RetReplacement::Block);
} }
}, },
MatchSource::IfDesugar {
contains_else_clause: true,
}
| MatchSource::IfLetDesugar {
contains_else_clause: true,
} => {
if let ExprKind::Block(ref ifblock, _) = arms[0].body.kind {
check_block_return(cx, ifblock);
}
check_final_expr(cx, arms[1].body, None, RetReplacement::Empty);
},
_ => (), _ => (),
} },
} _ => (),
fn emit_return_lint(cx: &EarlyContext<'_>, ret_span: Span, inner_span: Option<Span>, replacement: RetReplacement) {
match inner_span {
Some(inner_span) => {
if in_external_macro(cx.sess(), inner_span) || inner_span.from_expansion() {
return;
}
span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded `return` statement", |diag| {
if let Some(snippet) = snippet_opt(cx, inner_span) {
diag.span_suggestion(ret_span, "remove `return`", snippet, Applicability::MachineApplicable);
}
})
},
None => match replacement {
RetReplacement::Empty => {
span_lint_and_sugg(
cx,
NEEDLESS_RETURN,
ret_span,
"unneeded `return` statement",
"remove `return`",
String::new(),
Applicability::MachineApplicable,
);
},
RetReplacement::Block => {
span_lint_and_sugg(
cx,
NEEDLESS_RETURN,
ret_span,
"unneeded `return` statement",
"replace `return` with an empty block",
"{}".to_string(),
Applicability::MachineApplicable,
);
},
},
}
} }
} }
impl EarlyLintPass for Return { fn emit_return_lint(cx: &LateContext<'_>, ret_span: Span, inner_span: Option<Span>, replacement: RetReplacement) {
fn check_fn(&mut self, cx: &EarlyContext<'_>, kind: FnKind<'_>, span: Span, _: ast::NodeId) { match inner_span {
match kind { Some(inner_span) => {
FnKind::Fn(.., Some(block)) => self.check_block_return(cx, block), if in_external_macro(cx.tcx.sess, inner_span) || inner_span.from_expansion() {
FnKind::Closure(_, body) => self.check_final_expr(cx, body, Some(body.span), RetReplacement::Empty), return;
FnKind::Fn(.., None) => {},
}
if_chain! {
if let ast::FnRetTy::Ty(ref ty) = kind.decl().output;
if let ast::TyKind::Tup(ref vals) = ty.kind;
if vals.is_empty() && !ty.span.from_expansion() && get_def(span) == get_def(ty.span);
then {
lint_unneeded_unit_return(cx, ty, span);
} }
}
}
fn check_block(&mut self, cx: &EarlyContext<'_>, block: &ast::Block) { span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded `return` statement", |diag| {
if_chain! { if let Some(snippet) = snippet_opt(cx, inner_span) {
if let Some(ref stmt) = block.stmts.last(); diag.span_suggestion(ret_span, "remove `return`", snippet, Applicability::MachineApplicable);
if let ast::StmtKind::Expr(ref expr) = stmt.kind; }
if is_unit_expr(expr) && !stmt.span.from_expansion(); })
then { },
let sp = expr.span; None => match replacement {
RetReplacement::Empty => {
span_lint_and_sugg( span_lint_and_sugg(
cx, cx,
UNUSED_UNIT, NEEDLESS_RETURN,
sp, ret_span,
"unneeded unit expression", "unneeded `return` statement",
"remove the final `()`", "remove `return`",
String::new(), String::new(),
Applicability::MachineApplicable, Applicability::MachineApplicable,
); );
}
}
}
fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) {
match e.kind {
ast::ExprKind::Ret(Some(ref expr)) | ast::ExprKind::Break(_, Some(ref expr)) => {
if is_unit_expr(expr) && !expr.span.from_expansion() {
span_lint_and_sugg(
cx,
UNUSED_UNIT,
expr.span,
"unneeded `()`",
"remove the `()`",
String::new(),
Applicability::MachineApplicable,
);
}
}, },
_ => (), RetReplacement::Block => {
} span_lint_and_sugg(
} cx,
NEEDLESS_RETURN,
fn check_poly_trait_ref(&mut self, cx: &EarlyContext<'_>, poly: &ast::PolyTraitRef, _: &ast::TraitBoundModifier) { ret_span,
let segments = &poly.trait_ref.path.segments; "unneeded `return` statement",
"replace `return` with an empty block",
if_chain! { "{}".to_string(),
if segments.len() == 1;
if ["Fn", "FnMut", "FnOnce"].contains(&&*segments[0].ident.name.as_str());
if let Some(args) = &segments[0].args;
if let ast::GenericArgs::Parenthesized(generic_args) = &**args;
if let ast::FnRetTy::Ty(ty) = &generic_args.output;
if ty.kind.is_unit();
then {
lint_unneeded_unit_return(cx, ty, generic_args.span);
}
}
}
}
fn attr_is_cfg(attr: &ast::Attribute) -> bool {
attr.meta_item_list().is_some() && attr.has_name(sym!(cfg))
}
// get the def site
#[must_use]
fn get_def(span: Span) -> Option<Span> {
if span.from_expansion() {
Some(span.ctxt().outer_expn_data().def_site)
} else {
None
}
}
// is this expr a `()` unit?
fn is_unit_expr(expr: &ast::Expr) -> bool {
if let ast::ExprKind::Tup(ref vals) = expr.kind {
vals.is_empty()
} else {
false
}
}
fn lint_unneeded_unit_return(cx: &EarlyContext<'_>, ty: &ast::Ty, span: Span) {
let (ret_span, appl) = if let Ok(fn_source) = cx.sess().source_map().span_to_snippet(span.with_hi(ty.span.hi())) {
fn_source
.rfind("->")
.map_or((ty.span, Applicability::MaybeIncorrect), |rpos| {
(
#[allow(clippy::cast_possible_truncation)]
ty.span.with_lo(BytePos(span.lo().0 + rpos as u32)),
Applicability::MachineApplicable, Applicability::MachineApplicable,
) );
}) },
} else { },
(ty.span, Applicability::MaybeIncorrect) }
}; }
span_lint_and_sugg(
cx, fn last_statement_borrows<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
UNUSED_UNIT, let mut visitor = BorrowVisitor { cx, borrows: false };
ret_span, walk_expr(&mut visitor, expr);
"unneeded unit return type", visitor.borrows
"remove the `-> ()`", }
String::new(),
appl, struct BorrowVisitor<'a, 'tcx> {
); cx: &'a LateContext<'tcx>,
borrows: bool,
}
impl<'tcx> Visitor<'tcx> for BorrowVisitor<'_, 'tcx> {
type Map = Map<'tcx>;
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if self.borrows {
return;
}
if let Some(def_id) = fn_def_id(self.cx, expr) {
self.borrows = self
.cx
.tcx
.fn_sig(def_id)
.output()
.skip_binder()
.walk()
.any(|arg| matches!(arg.unpack(), GenericArgKind::Lifetime(_)));
}
walk_expr(self, expr);
}
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
} }

144
clippy_lints/src/unused_unit.rs Normal file
View file

@ -0,0 +1,144 @@
use if_chain::if_chain;
use rustc_ast::ast;
use rustc_ast::visit::FnKind;
use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;
use rustc_span::BytePos;
use crate::utils::span_lint_and_sugg;
declare_clippy_lint! {
/// **What it does:** Checks for unit (`()`) expressions that can be removed.
///
/// **Why is this bad?** Such expressions add no value, but can make the code
/// less readable. Depending on formatting they can make a `break` or `return`
/// statement look like a function call.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// fn return_unit() -> () {
/// ()
/// }
/// ```
pub UNUSED_UNIT,
style,
"needless unit expression"
}
declare_lint_pass!(UnusedUnit => [UNUSED_UNIT]);
impl EarlyLintPass for UnusedUnit {
fn check_fn(&mut self, cx: &EarlyContext<'_>, kind: FnKind<'_>, span: Span, _: ast::NodeId) {
if_chain! {
if let ast::FnRetTy::Ty(ref ty) = kind.decl().output;
if let ast::TyKind::Tup(ref vals) = ty.kind;
if vals.is_empty() && !ty.span.from_expansion() && get_def(span) == get_def(ty.span);
then {
lint_unneeded_unit_return(cx, ty, span);
}
}
}
fn check_block(&mut self, cx: &EarlyContext<'_>, block: &ast::Block) {
if_chain! {
if let Some(ref stmt) = block.stmts.last();
if let ast::StmtKind::Expr(ref expr) = stmt.kind;
if is_unit_expr(expr) && !stmt.span.from_expansion();
then {
let sp = expr.span;
span_lint_and_sugg(
cx,
UNUSED_UNIT,
sp,
"unneeded unit expression",
"remove the final `()`",
String::new(),
Applicability::MachineApplicable,
);
}
}
}
fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) {
match e.kind {
ast::ExprKind::Ret(Some(ref expr)) | ast::ExprKind::Break(_, Some(ref expr)) => {
if is_unit_expr(expr) && !expr.span.from_expansion() {
span_lint_and_sugg(
cx,
UNUSED_UNIT,
expr.span,
"unneeded `()`",
"remove the `()`",
String::new(),
Applicability::MachineApplicable,
);
}
},
_ => (),
}
}
fn check_poly_trait_ref(&mut self, cx: &EarlyContext<'_>, poly: &ast::PolyTraitRef, _: &ast::TraitBoundModifier) {
let segments = &poly.trait_ref.path.segments;
if_chain! {
if segments.len() == 1;
if ["Fn", "FnMut", "FnOnce"].contains(&&*segments[0].ident.name.as_str());
if let Some(args) = &segments[0].args;
if let ast::GenericArgs::Parenthesized(generic_args) = &**args;
if let ast::FnRetTy::Ty(ty) = &generic_args.output;
if ty.kind.is_unit();
then {
lint_unneeded_unit_return(cx, ty, generic_args.span);
}
}
}
}
// get the def site
#[must_use]
fn get_def(span: Span) -> Option<Span> {
if span.from_expansion() {
Some(span.ctxt().outer_expn_data().def_site)
} else {
None
}
}
// is this expr a `()` unit?
fn is_unit_expr(expr: &ast::Expr) -> bool {
if let ast::ExprKind::Tup(ref vals) = expr.kind {
vals.is_empty()
} else {
false
}
}
fn lint_unneeded_unit_return(cx: &EarlyContext<'_>, ty: &ast::Ty, span: Span) {
let (ret_span, appl) = if let Ok(fn_source) = cx.sess().source_map().span_to_snippet(span.with_hi(ty.span.hi())) {
fn_source
.rfind("->")
.map_or((ty.span, Applicability::MaybeIncorrect), |rpos| {
(
#[allow(clippy::cast_possible_truncation)]
ty.span.with_lo(BytePos(span.lo().0 + rpos as u32)),
Applicability::MachineApplicable,
)
})
} else {
(ty.span, Applicability::MaybeIncorrect)
};
span_lint_and_sugg(
cx,
UNUSED_UNIT,
ret_span,
"unneeded unit return type",
"remove the `-> ()`",
String::new(),
appl,
);
}

4
src/lintlist/mod.rs
View file

@ -1037,7 +1037,7 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
group: "style", group: "style",
desc: "creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block", desc: "creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block",
deprecation: None, deprecation: None,
module: "let_and_return", module: "returns",
}, },
Lint { Lint {
name: "let_underscore_lock", name: "let_underscore_lock",
@ -2493,7 +2493,7 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
group: "style", group: "style",
desc: "needless unit expression", desc: "needless unit expression",
deprecation: None, deprecation: None,
module: "returns", module: "unused_unit",
}, },
Lint { Lint {
name: "unwrap_used", name: "unwrap_used",

17
tests/ui/needless_return.fixed
View file

@ -69,6 +69,23 @@ fn test_void_match(x: u32) {
} }
} }
fn read_line() -> String {
use std::io::BufRead;
let stdin = ::std::io::stdin();
return stdin.lock().lines().next().unwrap().unwrap();
}
fn borrows_but_not_last(value: bool) -> String {
if value {
use std::io::BufRead;
let stdin = ::std::io::stdin();
let _a = stdin.lock().lines().next().unwrap().unwrap();
String::from("test")
} else {
String::new()
}
}
fn main() { fn main() {
let _ = test_end_of_fn(); let _ = test_end_of_fn();
let _ = test_no_semicolon(); let _ = test_no_semicolon();

17
tests/ui/needless_return.rs
View file

@ -69,6 +69,23 @@ fn test_void_match(x: u32) {
} }
} }
fn read_line() -> String {
use std::io::BufRead;
let stdin = ::std::io::stdin();
return stdin.lock().lines().next().unwrap().unwrap();
}
fn borrows_but_not_last(value: bool) -> String {
if value {
use std::io::BufRead;
let stdin = ::std::io::stdin();
let _a = stdin.lock().lines().next().unwrap().unwrap();
return String::from("test");
} else {
return String::new();
}
}
fn main() { fn main() {
let _ = test_end_of_fn(); let _ = test_end_of_fn();
let _ = test_no_semicolon(); let _ = test_no_semicolon();

14
tests/ui/needless_return.stderr
View file

@ -72,5 +72,17 @@ error: unneeded `return` statement
LL | _ => return, LL | _ => return,
| ^^^^^^ help: replace `return` with an empty block: `{}` | ^^^^^^ help: replace `return` with an empty block: `{}`
error: aborting due to 12 previous errors error: unneeded `return` statement
--> $DIR/needless_return.rs:83:9
|
LL | return String::from("test");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::from("test")`
error: unneeded `return` statement
--> $DIR/needless_return.rs:85:9
|
LL | return String::new();
| ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::new()`
error: aborting due to 14 previous errors