summaryrefslogtreecommitdiffstats
path: root/src/tools/clippy/clippy_lints/src/returns.rs
diff options
context:
space:
mode:
Diffstat (limited to '')
-rw-r--r--src/tools/clippy/clippy_lints/src/returns.rs212
1 files changed, 89 insertions, 123 deletions
diff --git a/src/tools/clippy/clippy_lints/src/returns.rs b/src/tools/clippy/clippy_lints/src/returns.rs
index 91553240e..2b2a41d16 100644
--- a/src/tools/clippy/clippy_lints/src/returns.rs
+++ b/src/tools/clippy/clippy_lints/src/returns.rs
@@ -1,9 +1,11 @@
-use clippy_utils::diagnostics::span_lint_hir_and_then;
+use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
use clippy_utils::source::{snippet_opt, snippet_with_context};
+use clippy_utils::visitors::{for_each_expr, Descend};
use clippy_utils::{fn_def_id, path_to_local_id};
+use core::ops::ControlFlow;
use if_chain::if_chain;
use rustc_errors::Applicability;
-use rustc_hir::intravisit::{walk_expr, FnKind, Visitor};
+use rustc_hir::intravisit::FnKind;
use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl, HirId, MatchSource, PatKind, StmtKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
@@ -72,6 +74,27 @@ enum RetReplacement {
Unit,
}
+impl RetReplacement {
+ fn sugg_help(self) -> &'static str {
+ match self {
+ Self::Empty => "remove `return`",
+ Self::Block => "replace `return` with an empty block",
+ Self::Unit => "replace `return` with a unit value",
+ }
+ }
+}
+
+impl ToString for RetReplacement {
+ fn to_string(&self) -> String {
+ match *self {
+ Self::Empty => "",
+ Self::Block => "{}",
+ Self::Unit => "()",
+ }
+ .to_string()
+ }
+}
+
declare_lint_pass!(Return => [LET_AND_RETURN, NEEDLESS_RETURN]);
impl<'tcx> LateLintPass<'tcx> for Return {
@@ -139,26 +162,35 @@ impl<'tcx> LateLintPass<'tcx> for Return {
} else {
RetReplacement::Empty
};
- check_final_expr(cx, body.value, Some(body.value.span), replacement);
+ check_final_expr(cx, body.value, vec![], replacement);
},
FnKind::ItemFn(..) | FnKind::Method(..) => {
- if let ExprKind::Block(block, _) = body.value.kind {
- check_block_return(cx, block);
- }
+ check_block_return(cx, &body.value.kind, vec![]);
},
}
}
}
-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(expr) | StmtKind::Semi(expr) => {
- check_final_expr(cx, expr, Some(stmt.span), RetReplacement::Empty);
- },
- _ => (),
+// if `expr` is a block, check if there are needless returns in it
+fn check_block_return<'tcx>(cx: &LateContext<'tcx>, expr_kind: &ExprKind<'tcx>, semi_spans: Vec<Span>) {
+ if let ExprKind::Block(block, _) = expr_kind {
+ if let Some(block_expr) = block.expr {
+ check_final_expr(cx, block_expr, semi_spans, RetReplacement::Empty);
+ } else if let Some(stmt) = block.stmts.iter().last() {
+ match stmt.kind {
+ StmtKind::Expr(expr) => {
+ check_final_expr(cx, expr, semi_spans, RetReplacement::Empty);
+ },
+ StmtKind::Semi(semi_expr) => {
+ let mut semi_spans_and_this_one = semi_spans;
+ // we only want the span containing the semicolon so we can remove it later. From `entry.rs:382`
+ if let Some(semicolon_span) = stmt.span.trim_start(semi_expr.span) {
+ semi_spans_and_this_one.push(semicolon_span);
+ check_final_expr(cx, semi_expr, semi_spans_and_this_one, RetReplacement::Empty);
+ }
+ },
+ _ => (),
+ }
}
}
}
@@ -166,10 +198,12 @@ fn check_block_return<'tcx>(cx: &LateContext<'tcx>, block: &Block<'tcx>) {
fn check_final_expr<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'tcx>,
- span: Option<Span>,
+ semi_spans: Vec<Span>, /* containing all the places where we would need to remove semicolons if finding an
+ * needless return */
replacement: RetReplacement,
) {
- match expr.kind {
+ let peeled_drop_expr = expr.peel_drop_temps();
+ match &peeled_drop_expr.kind {
// simple return is always "bad"
ExprKind::Ret(ref inner) => {
if cx.tcx.hir().attrs(expr.hir_id).is_empty() {
@@ -177,24 +211,18 @@ fn check_final_expr<'tcx>(
if !borrows {
emit_return_lint(
cx,
- inner.map_or(expr.hir_id, |inner| inner.hir_id),
- span.expect("`else return` is not possible"),
+ peeled_drop_expr.span,
+ semi_spans,
inner.as_ref().map(|i| i.span),
replacement,
);
}
}
},
- // a whole block? check it!
- ExprKind::Block(block, _) => {
- check_block_return(cx, block);
- },
ExprKind::If(_, then, else_clause_opt) => {
- if let ExprKind::Block(ifblock, _) = then.kind {
- check_block_return(cx, ifblock);
- }
+ check_block_return(cx, &then.kind, semi_spans.clone());
if let Some(else_clause) = else_clause_opt {
- check_final_expr(cx, else_clause, None, RetReplacement::Empty);
+ check_block_return(cx, &else_clause.kind, semi_spans);
}
},
// a match expr, check all arms
@@ -203,123 +231,61 @@ fn check_final_expr<'tcx>(
// (except for unit type functions) so we don't match it
ExprKind::Match(_, arms, MatchSource::Normal) => {
for arm in arms.iter() {
- check_final_expr(cx, arm.body, Some(arm.body.span), RetReplacement::Unit);
+ check_final_expr(cx, arm.body, semi_spans.clone(), RetReplacement::Unit);
}
},
- ExprKind::DropTemps(expr) => check_final_expr(cx, expr, None, RetReplacement::Empty),
- _ => (),
+ // if it's a whole block, check it
+ other_expr_kind => check_block_return(cx, other_expr_kind, semi_spans),
}
}
fn emit_return_lint(
cx: &LateContext<'_>,
- emission_place: HirId,
ret_span: Span,
+ semi_spans: Vec<Span>,
inner_span: Option<Span>,
replacement: RetReplacement,
) {
if ret_span.from_expansion() {
return;
}
- match inner_span {
- Some(inner_span) => {
- let mut applicability = Applicability::MachineApplicable;
- span_lint_hir_and_then(
- cx,
- NEEDLESS_RETURN,
- emission_place,
- ret_span,
- "unneeded `return` statement",
- |diag| {
- let (snippet, _) = snippet_with_context(cx, inner_span, ret_span.ctxt(), "..", &mut applicability);
- diag.span_suggestion(ret_span, "remove `return`", snippet, applicability);
- },
- );
- },
- None => match replacement {
- RetReplacement::Empty => {
- span_lint_hir_and_then(
- cx,
- NEEDLESS_RETURN,
- emission_place,
- ret_span,
- "unneeded `return` statement",
- |diag| {
- diag.span_suggestion(
- ret_span,
- "remove `return`",
- String::new(),
- Applicability::MachineApplicable,
- );
- },
- );
- },
- RetReplacement::Block => {
- span_lint_hir_and_then(
- cx,
- NEEDLESS_RETURN,
- emission_place,
- ret_span,
- "unneeded `return` statement",
- |diag| {
- diag.span_suggestion(
- ret_span,
- "replace `return` with an empty block",
- "{}".to_string(),
- Applicability::MachineApplicable,
- );
- },
- );
- },
- RetReplacement::Unit => {
- span_lint_hir_and_then(
- cx,
- NEEDLESS_RETURN,
- emission_place,
- ret_span,
- "unneeded `return` statement",
- |diag| {
- diag.span_suggestion(
- ret_span,
- "replace `return` with a unit value",
- "()".to_string(),
- Applicability::MachineApplicable,
- );
- },
- );
- },
+ let mut applicability = Applicability::MachineApplicable;
+ let return_replacement = inner_span.map_or_else(
+ || replacement.to_string(),
+ |inner_span| {
+ let (snippet, _) = snippet_with_context(cx, inner_span, ret_span.ctxt(), "..", &mut applicability);
+ snippet.to_string()
},
- }
+ );
+ let sugg_help = if inner_span.is_some() {
+ "remove `return`"
+ } else {
+ replacement.sugg_help()
+ };
+ span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded `return` statement", |diag| {
+ diag.span_suggestion_hidden(ret_span, sugg_help, return_replacement, applicability);
+ // for each parent statement, we need to remove the semicolon
+ for semi_stmt_span in semi_spans {
+ diag.tool_only_span_suggestion(semi_stmt_span, "remove this semicolon", "", applicability);
+ }
+ });
}
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> {
- fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
- if self.borrows || expr.span.from_expansion() {
- return;
- }
-
- if let Some(def_id) = fn_def_id(self.cx, expr) {
- self.borrows = self
- .cx
+ for_each_expr(expr, |e| {
+ if let Some(def_id) = fn_def_id(cx, e)
+ && cx
.tcx
.fn_sig(def_id)
- .output()
.skip_binder()
+ .output()
.walk()
- .any(|arg| matches!(arg.unpack(), GenericArgKind::Lifetime(_)));
+ .any(|arg| matches!(arg.unpack(), GenericArgKind::Lifetime(_)))
+ {
+ ControlFlow::Break(())
+ } else {
+ ControlFlow::Continue(Descend::from(!expr.span.from_expansion()))
}
-
- walk_expr(self, expr);
- }
+ })
+ .is_some()
}
generated by cgit v1.2.3 (git 2.39.1) at 2024-07-26 19:42:41 +0000