Skip to content

[needless_return]: Remove all semicolons on suggestion #10187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 14 additions & 12 deletions clippy_lints/src/returns.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
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 clippy_utils::{fn_def_id, path_to_local_id, span_find_starting_semi};
use core::ops::ControlFlow;
use if_chain::if_chain;
use rustc_errors::Applicability;
Expand Down Expand Up @@ -151,7 +151,7 @@ impl<'tcx> LateLintPass<'tcx> for Return {
kind: FnKind<'tcx>,
_: &'tcx FnDecl<'tcx>,
body: &'tcx Body<'tcx>,
_: Span,
sp: Span,
_: HirId,
) {
match kind {
Expand All @@ -166,14 +166,14 @@ impl<'tcx> LateLintPass<'tcx> for Return {
check_final_expr(cx, body.value, vec![], replacement);
},
FnKind::ItemFn(..) | FnKind::Method(..) => {
check_block_return(cx, &body.value.kind, vec![]);
check_block_return(cx, &body.value.kind, sp, vec![]);
},
}
}
}

// 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>) {
fn check_block_return<'tcx>(cx: &LateContext<'tcx>, expr_kind: &ExprKind<'tcx>, sp: Span, mut 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);
Expand All @@ -183,12 +183,14 @@ fn check_block_return<'tcx>(cx: &LateContext<'tcx>, expr_kind: &ExprKind<'tcx>,
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);
// Remove ending semicolons and any whitespace ' ' in between.
// Without `return`, the suggestion might not compile if the semicolon is retained
if let Some(semi_span) = stmt.span.trim_start(semi_expr.span) {
let semi_span_to_remove =
span_find_starting_semi(cx.sess().source_map(), semi_span.with_hi(sp.hi()));
semi_spans.push(semi_span_to_remove);
}
check_final_expr(cx, semi_expr, semi_spans, RetReplacement::Empty);
},
_ => (),
}
Expand Down Expand Up @@ -231,9 +233,9 @@ fn check_final_expr<'tcx>(
emit_return_lint(cx, ret_span, semi_spans, inner.as_ref().map(|i| i.span), replacement);
},
ExprKind::If(_, then, else_clause_opt) => {
check_block_return(cx, &then.kind, semi_spans.clone());
check_block_return(cx, &then.kind, peeled_drop_expr.span, semi_spans.clone());
if let Some(else_clause) = else_clause_opt {
check_block_return(cx, &else_clause.kind, semi_spans);
check_block_return(cx, &else_clause.kind, peeled_drop_expr.span, semi_spans);
}
},
// a match expr, check all arms
Expand All @@ -246,7 +248,7 @@ fn check_final_expr<'tcx>(
}
},
// if it's a whole block, check it
other_expr_kind => check_block_return(cx, other_expr_kind, semi_spans),
other_expr_kind => check_block_return(cx, other_expr_kind, peeled_drop_expr.span, semi_spans),
}
}

Expand Down
4 changes: 4 additions & 0 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2488,6 +2488,10 @@ pub fn span_extract_comment(sm: &SourceMap, span: Span) -> String {
comments_buf.join("\n")
}

pub fn span_find_starting_semi(sm: &SourceMap, span: Span) -> Span {
sm.span_take_while(span, |&ch| ch == ' ' || ch == ';')
}

macro_rules! op_utils {
($($name:ident $assign:ident)*) => {
/// Binary operation traits like `LangItem::Add`
Expand Down
10 changes: 10 additions & 0 deletions tests/ui/needless_return.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,16 @@ fn test_no_semicolon() -> bool {
true
}

#[rustfmt::skip]
fn test_multiple_semicolon() -> bool {
true
}

#[rustfmt::skip]
fn test_multiple_semicolon_with_spaces() -> bool {
true
}

fn test_if_block() -> bool {
if true {
true
Expand Down
10 changes: 10 additions & 0 deletions tests/ui/needless_return.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,16 @@ fn test_no_semicolon() -> bool {
return true;
}

#[rustfmt::skip]
fn test_multiple_semicolon() -> bool {
return true;;;
}

#[rustfmt::skip]
fn test_multiple_semicolon_with_spaces() -> bool {
return true;; ; ;
}

fn test_if_block() -> bool {
if true {
return true;
Expand Down
Loading