From 0cd9b06125c5a4ffce659bc92328946be94ccc2b Mon Sep 17 00:00:00 2001 From: Niki4tap Date: Fri, 23 Dec 2022 00:56:50 +0300 Subject: [PATCH 1/3] Small code style adjustments --- clippy_lints/src/returns.rs | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index d4d50660520..0da50450512 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -210,22 +210,25 @@ fn check_final_expr<'tcx>( // if desugar of `do yeet`, don't lint if let Some(inner_expr) = inner && let ExprKind::Call(path_expr, _) = inner_expr.kind - && let ExprKind::Path(QPath::LangItem(LangItem::TryTraitFromYeet, _, _)) = path_expr.kind { - return; + && let ExprKind::Path(QPath::LangItem(LangItem::TryTraitFromYeet, _, _)) = path_expr.kind + { + return; } - if cx.tcx.hir().attrs(expr.hir_id).is_empty() { - let borrows = inner.map_or(false, |inner| last_statement_borrows(cx, inner)); - if !borrows { - // check if expr return nothing - let ret_span = if inner.is_none() && replacement == RetReplacement::Empty { - extend_span_to_previous_non_ws(cx, peeled_drop_expr.span) - } else { - peeled_drop_expr.span - }; + if !cx.tcx.hir().attrs(expr.hir_id).is_empty() { + return; + } + let borrows = inner.map_or(false, |inner| last_statement_borrows(cx, inner)); + if borrows { + return; + } + // check if expr return nothing + let ret_span = if inner.is_none() && replacement == RetReplacement::Empty { + extend_span_to_previous_non_ws(cx, peeled_drop_expr.span) + } else { + peeled_drop_expr.span + }; - emit_return_lint(cx, ret_span, semi_spans, inner.as_ref().map(|i| i.span), replacement); - } - } + 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()); From a9358260717e55c367f16bc033990bac77cfb5a1 Mon Sep 17 00:00:00 2001 From: Niki4tap Date: Fri, 23 Dec 2022 00:57:28 +0300 Subject: [PATCH 2/3] Fix the actual bug --- clippy_lints/src/returns.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index 0da50450512..bbbd9e4989e 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -295,7 +295,7 @@ fn last_statement_borrows<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { ControlFlow::Break(()) } else { - ControlFlow::Continue(Descend::from(!expr.span.from_expansion())) + ControlFlow::Continue(Descend::from(!e.span.from_expansion())) } }) .is_some() From 9ff868c4ae90f2b94d086cf8cf6302118c29f2ee Mon Sep 17 00:00:00 2001 From: Niki4tap Date: Fri, 23 Dec 2022 01:14:07 +0300 Subject: [PATCH 3/3] test test test --- tests/ui/needless_return.fixed | 10 ++++++++++ tests/ui/needless_return.rs | 10 ++++++++++ tests/ui/needless_return.stderr | 18 +++++++++++++++++- 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/tests/ui/needless_return.fixed b/tests/ui/needless_return.fixed index d451be1f389..ab1c0e590bb 100644 --- a/tests/ui/needless_return.fixed +++ b/tests/ui/needless_return.fixed @@ -277,4 +277,14 @@ fn issue9947() -> Result<(), String> { do yeet "hello"; } +// without anyhow, but triggers the same bug I believe +#[expect(clippy::useless_format)] +fn issue10051() -> Result { + if true { + Ok(format!("ok!")) + } else { + Err(format!("err!")) + } +} + fn main() {} diff --git a/tests/ui/needless_return.rs b/tests/ui/needless_return.rs index e1a1bea2c0b..abed338bb9b 100644 --- a/tests/ui/needless_return.rs +++ b/tests/ui/needless_return.rs @@ -287,4 +287,14 @@ fn issue9947() -> Result<(), String> { do yeet "hello"; } +// without anyhow, but triggers the same bug I believe +#[expect(clippy::useless_format)] +fn issue10051() -> Result { + if true { + return Ok(format!("ok!")); + } else { + return Err(format!("err!")); + } +} + fn main() {} diff --git a/tests/ui/needless_return.stderr b/tests/ui/needless_return.stderr index ca2253e6586..52eabf6e137 100644 --- a/tests/ui/needless_return.stderr +++ b/tests/ui/needless_return.stderr @@ -386,5 +386,21 @@ LL | let _ = 42; return; | = help: remove `return` -error: aborting due to 46 previous errors +error: unneeded `return` statement + --> $DIR/needless_return.rs:294:9 + | +LL | return Ok(format!("ok!")); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: remove `return` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:296:9 + | +LL | return Err(format!("err!")); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: remove `return` + +error: aborting due to 48 previous errors