Skip to content

Commit f23ec55

Browse files
Centri3Centri3
committed
New lint needless_return_with_try 
1 parent b10a0aa commit f23ec55

File tree

9 files changed

+184
-16
lines changed

9 files changed

+184
-16
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5092,6 +5092,7 @@ Released 2018-09-13
50925092
[`needless_raw_string_hashes`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_raw_string_hashes
50935093
[`needless_raw_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_raw_strings
50945094
[`needless_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
5095+
[`needless_return_with_try`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_return_with_try
50955096
[`needless_splitn`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_splitn
50965097
[`needless_update`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_update
50975098
[`neg_cmp_op_on_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#neg_cmp_op_on_partial_ord

‎clippy_lints/src/declared_lints.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -569,6 +569,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
569569
crate::return_self_not_must_use::RETURN_SELF_NOT_MUST_USE_INFO,
570570
crate::returns::LET_AND_RETURN_INFO,
571571
crate::returns::NEEDLESS_RETURN_INFO,
572+
crate::returns::NEEDLESS_RETURN_WITH_TRY_INFO,
572573
crate::same_name_method::SAME_NAME_METHOD_INFO,
573574
crate::self_named_constructors::SELF_NAMED_CONSTRUCTORS_INFO,
574575
crate::semicolon_block::SEMICOLON_INSIDE_BLOCK_INFO,

‎clippy_lints/src/returns.rs‎

Lines changed: 71 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
1-
use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
1+
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then, span_lint_hir_and_then};
22
use clippy_utils::source::{snippet_opt, snippet_with_context};
33
use clippy_utils::visitors::{for_each_expr_with_closures, Descend};
44
use clippy_utils::{fn_def_id, path_to_local_id, span_find_starting_semi};
55
use core::ops::ControlFlow;
66
use if_chain::if_chain;
77
use rustc_errors::Applicability;
88
use rustc_hir::intravisit::FnKind;
9-
use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl, LangItem, MatchSource, PatKind, QPath, StmtKind};
9+
use rustc_hir::{
10+
Block, Body, Expr, ExprKind, FnDecl, ItemKind, LangItem, MatchSource, OwnerNode, PatKind, QPath, Stmt, StmtKind,
11+
};
1012
use rustc_lint::{LateContext, LateLintPass, LintContext};
1113
use rustc_middle::lint::in_external_macro;
1214
use rustc_middle::ty::{self, subst::GenericArgKind, Ty};
@@ -76,6 +78,46 @@ declare_clippy_lint! {
7678
"using a return statement like `return expr;` where an expression would suffice"
7779
}
7880

81+
declare_clippy_lint! {
82+
/// ### What it does
83+
/// Checks for return statements on `Err` paired with the `?` operator.
84+
///
85+
/// ### Why is this bad?
86+
/// The `return` is unnecessary.
87+
///
88+
/// ### Example
89+
/// ```rust,ignore
90+
/// fn foo(x: usize) -> Result<(), Box<dyn Error>> {
91+
/// if x == 0 {
92+
/// return Err(...)?;
93+
/// }
94+
/// Ok(())
95+
/// }
96+
/// ```
97+
/// simplify to
98+
/// ```rust,ignore
99+
/// fn foo(x: usize) -> Result<(), Box<dyn Error>> {
100+
/// if x == 0 {
101+
/// Err(...)?;
102+
/// }
103+
/// Ok(())
104+
/// }
105+
/// ```
106+
/// if paired with `try_err`, use instead:
107+
/// ```rust,ignore
108+
/// fn foo(x: usize) -> Result<(), Box<dyn Error>> {
109+
/// if x == 0 {
110+
/// return Err(...);
111+
/// }
112+
/// Ok(())
113+
/// }
114+
/// ```
115+
#[clippy::version = "1.72.0"]
116+
pub NEEDLESS_RETURN_WITH_TRY,
117+
style,
118+
"using a return statement like `return Err(expr)?;` where removing it would suffice"
119+
}
120+
79121
#[derive(PartialEq, Eq)]
80122
enum RetReplacement<'tcx> {
81123
Empty,
@@ -115,9 +157,35 @@ impl<'tcx> ToString for RetReplacement<'tcx> {
115157
}
116158
}
117159

118-
declare_lint_pass!(Return => [LET_AND_RETURN, NEEDLESS_RETURN]);
160+
declare_lint_pass!(Return => [LET_AND_RETURN, NEEDLESS_RETURN, NEEDLESS_RETURN_WITH_TRY]);
119161

120162
impl<'tcx> LateLintPass<'tcx> for Return {
163+
fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) {
164+
if !in_external_macro(cx.sess(), stmt.span)
165+
&& let StmtKind::Semi(expr) = stmt.kind
166+
&& let ExprKind::Ret(Some(ret)) = expr.kind
167+
&& let ExprKind::Match(.., MatchSource::TryDesugar) = ret.kind
168+
// Ensure this is not the final stmt, otherwise removing it would cause a compile error
169+
&& let OwnerNode::Item(item) = cx.tcx.hir().owner(cx.tcx.hir().get_parent_item(expr.hir_id))
170+
&& let ItemKind::Fn(_, _, body) = item.kind
171+
&& let block = cx.tcx.hir().body(body).value
172+
&& let ExprKind::Block(block, _) = block.kind
173+
&& let [.., final_stmt] = block.stmts
174+
&& final_stmt.hir_id != stmt.hir_id
175+
&& !is_from_proc_macro(cx, expr)
176+
{
177+
span_lint_and_sugg(
178+
cx,
179+
NEEDLESS_RETURN_WITH_TRY,
180+
expr.span.until(ret.span),
181+
"unneeded `return` statement with `?` operator",
182+
"remove it",
183+
String::new(),
184+
Applicability::MachineApplicable,
185+
);
186+
}
187+
}
188+
121189
fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'_>) {
122190
// we need both a let-binding stmt and an expr
123191
if_chain! {

‎tests/ui/needless_return_with_try.fixed‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
//@run-rustfix
2+
//@aux-build:proc_macros.rs
3+
#![allow(
4+
clippy::needless_return,
5+
clippy::no_effect,
6+
clippy::unit_arg,
7+
clippy::useless_conversion,
8+
unused
9+
)]
10+
11+
#[macro_use]
12+
extern crate proc_macros;
13+
14+
fn a() -> u32 {
15+
return 0;
16+
}
17+
18+
fn b() -> Result<u32, u32> {
19+
return Err(0);
20+
}
21+
22+
// Do not lint
23+
fn c() -> Option<()> {
24+
return None?;
25+
}
26+
27+
fn main() -> Result<(), ()> {
28+
Err(())?;
29+
return Ok::<(), ()>(());
30+
Err(())?;
31+
Ok::<(), ()>(());
32+
return Err(().into());
33+
external! {
34+
return Err(())?;
35+
}
36+
with_span! {
37+
return Err(())?;
38+
}
39+
Err(())
40+
}

‎tests/ui/needless_return_with_try.rs‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
//@run-rustfix
2+
//@aux-build:proc_macros.rs
3+
#![allow(
4+
clippy::needless_return,
5+
clippy::no_effect,
6+
clippy::unit_arg,
7+
clippy::useless_conversion,
8+
unused
9+
)]
10+
11+
#[macro_use]
12+
extern crate proc_macros;
13+
14+
fn a() -> u32 {
15+
return 0;
16+
}
17+
18+
fn b() -> Result<u32, u32> {
19+
return Err(0);
20+
}
21+
22+
// Do not lint
23+
fn c() -> Option<()> {
24+
return None?;
25+
}
26+
27+
fn main() -> Result<(), ()> {
28+
return Err(())?;
29+
return Ok::<(), ()>(());
30+
Err(())?;
31+
Ok::<(), ()>(());
32+
return Err(().into());
33+
external! {
34+
return Err(())?;
35+
}
36+
with_span! {
37+
return Err(())?;
38+
}
39+
Err(())
40+
}

‎tests/ui/needless_return_with_try.stderr‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
error: unneeded `return` statement with `?` operator
2+
--> $DIR/needless_return_with_try.rs:28:5
3+
|
4+
LL | return Err(())?;
5+
| ^^^^^^^ help: remove it
6+
|
7+
= note: `-D clippy::needless-return-with-try` implied by `-D warnings`
8+
9+
error: aborting due to previous error
10+

‎tests/ui/try_err.fixed‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@
22
//@aux-build:proc_macros.rs:proc-macro
33

44
#![deny(clippy::try_err)]
5-
#![allow(clippy::unnecessary_wraps, clippy::needless_question_mark)]
5+
#![allow(
6+
clippy::unnecessary_wraps,
7+
clippy::needless_question_mark,
8+
clippy::needless_return_with_try
9+
)]
610

711
extern crate proc_macros;
812
use proc_macros::{external, inline_macros};

‎tests/ui/try_err.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@
22
//@aux-build:proc_macros.rs:proc-macro
33

44
#![deny(clippy::try_err)]
5-
#![allow(clippy::unnecessary_wraps, clippy::needless_question_mark)]
5+
#![allow(
6+
clippy::unnecessary_wraps,
7+
clippy::needless_question_mark,
8+
clippy::needless_return_with_try
9+
)]
610

711
extern crate proc_macros;
812
use proc_macros::{external, inline_macros};

‎tests/ui/try_err.stderr‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: returning an `Err(_)` with the `?` operator
2-
--> $DIR/try_err.rs:19:9
2+
--> $DIR/try_err.rs:23:9
33
|
44
LL | Err(err)?;
55
| ^^^^^^^^^ help: try: `return Err(err)`
@@ -11,65 +11,65 @@ LL | #![deny(clippy::try_err)]
1111
| ^^^^^^^^^^^^^^^
1212

1313
error: returning an `Err(_)` with the `?` operator
14-
--> $DIR/try_err.rs:29:9
14+
--> $DIR/try_err.rs:33:9
1515
|
1616
LL | Err(err)?;
1717
| ^^^^^^^^^ help: try: `return Err(err.into())`
1818

1919
error: returning an `Err(_)` with the `?` operator
20-
--> $DIR/try_err.rs:49:17
20+
--> $DIR/try_err.rs:53:17
2121
|
2222
LL | Err(err)?;
2323
| ^^^^^^^^^ help: try: `return Err(err)`
2424

2525
error: returning an `Err(_)` with the `?` operator
26-
--> $DIR/try_err.rs:68:17
26+
--> $DIR/try_err.rs:72:17
2727
|
2828
LL | Err(err)?;
2929
| ^^^^^^^^^ help: try: `return Err(err.into())`
3030

3131
error: returning an `Err(_)` with the `?` operator
32-
--> $DIR/try_err.rs:88:23
32+
--> $DIR/try_err.rs:92:23
3333
|
3434
LL | Err(_) => Err(1)?,
3535
| ^^^^^^^ help: try: `return Err(1)`
3636
|
3737
= note: this error originates in the macro `__inline_mac_fn_calling_macro` (in Nightly builds, run with -Z macro-backtrace for more info)
3838

3939
error: returning an `Err(_)` with the `?` operator
40-
--> $DIR/try_err.rs:95:23
40+
--> $DIR/try_err.rs:99:23
4141
|
4242
LL | Err(_) => Err(inline!(1))?,
4343
| ^^^^^^^^^^^^^^^^ help: try: `return Err(inline!(1))`
4444
|
4545
= note: this error originates in the macro `__inline_mac_fn_calling_macro` (in Nightly builds, run with -Z macro-backtrace for more info)
4646

4747
error: returning an `Err(_)` with the `?` operator
48-
--> $DIR/try_err.rs:122:9
48+
--> $DIR/try_err.rs:126:9
4949
|
5050
LL | Err(inline!(inline!(String::from("aasdfasdfasdfa"))))?;
5151
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `return Err(inline!(inline!(String::from("aasdfasdfasdfa"))))`
5252

5353
error: returning an `Err(_)` with the `?` operator
54-
--> $DIR/try_err.rs:129:9
54+
--> $DIR/try_err.rs:133:9
5555
|
5656
LL | Err(io::ErrorKind::WriteZero)?
5757
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `return Poll::Ready(Err(io::ErrorKind::WriteZero.into()))`
5858

5959
error: returning an `Err(_)` with the `?` operator
60-
--> $DIR/try_err.rs:131:9
60+
--> $DIR/try_err.rs:135:9
6161
|
6262
LL | Err(io::Error::new(io::ErrorKind::InvalidInput, "error"))?
6363
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `return Poll::Ready(Err(io::Error::new(io::ErrorKind::InvalidInput, "error")))`
6464

6565
error: returning an `Err(_)` with the `?` operator
66-
--> $DIR/try_err.rs:139:9
66+
--> $DIR/try_err.rs:143:9
6767
|
6868
LL | Err(io::ErrorKind::NotFound)?
6969
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `return Poll::Ready(Some(Err(io::ErrorKind::NotFound.into())))`
7070

7171
error: returning an `Err(_)` with the `?` operator
72-
--> $DIR/try_err.rs:148:16
72+
--> $DIR/try_err.rs:152:16
7373
|
7474
LL | return Err(42)?;
7575
| ^^^^^^^^ help: try: `Err(42)`

0 commit comments

Comments
 (0)