needless_return: Support #[expect] on the return statement

This commit is contained in:
xFrednet 2024-07-01 13:49:16 +02:00
parent f24a87093e
commit 903874d2f4
No known key found for this signature in database
GPG Key ID: F5C59D0E669E5302
4 changed files with 116 additions and 29 deletions

View File

@ -7,6 +7,7 @@ use clippy_utils::{
path_to_local_id, span_contains_cfg, span_find_starting_semi, path_to_local_id, span_contains_cfg, span_find_starting_semi,
}; };
use core::ops::ControlFlow; use core::ops::ControlFlow;
use rustc_ast::NestedMetaItem;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::intravisit::FnKind; use rustc_hir::intravisit::FnKind;
use rustc_hir::LangItem::ResultErr; use rustc_hir::LangItem::ResultErr;
@ -14,13 +15,13 @@ use rustc_hir::{
Block, Body, Expr, ExprKind, FnDecl, HirId, ItemKind, LangItem, MatchSource, Node, OwnerNode, PatKind, QPath, Stmt, Block, Body, Expr, ExprKind, FnDecl, HirId, ItemKind, LangItem, MatchSource, Node, OwnerNode, PatKind, QPath, Stmt,
StmtKind, StmtKind,
}; };
use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_lint::{LateContext, LateLintPass, Level, LintContext};
use rustc_middle::lint::in_external_macro; use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::adjustment::Adjust; use rustc_middle::ty::adjustment::Adjust;
use rustc_middle::ty::{self, GenericArgKind, Ty}; use rustc_middle::ty::{self, GenericArgKind, Ty};
use rustc_session::declare_lint_pass; use rustc_session::declare_lint_pass;
use rustc_span::def_id::LocalDefId; use rustc_span::def_id::LocalDefId;
use rustc_span::{BytePos, Pos, Span}; use rustc_span::{sym, BytePos, Pos, Span};
use std::borrow::Cow; use std::borrow::Cow;
use std::fmt::Display; use std::fmt::Display;
@ -80,6 +81,9 @@ declare_clippy_lint! {
/// ``` /// ```
#[clippy::version = "pre 1.29.0"] #[clippy::version = "pre 1.29.0"]
pub NEEDLESS_RETURN, pub NEEDLESS_RETURN,
// This lint requires some special handling in `check_final_expr` for `#[expect]`.
// This handling needs to be updated if the group gets changed. This should also
// be caught by tests.
style, style,
"using a return statement like `return expr;` where an expression would suffice" "using a return statement like `return expr;` where an expression would suffice"
} }
@ -91,6 +95,9 @@ declare_clippy_lint! {
/// ### Why is this bad? /// ### Why is this bad?
/// The `return` is unnecessary. /// The `return` is unnecessary.
/// ///
/// Returns may be used to add attributes to the return expression. Return
/// statements with attributes are therefore be accepted by this lint.
///
/// ### Example /// ### Example
/// ```rust,ignore /// ```rust,ignore
/// fn foo(x: usize) -> Result<(), Box<dyn Error>> { /// fn foo(x: usize) -> Result<(), Box<dyn Error>> {
@ -377,13 +384,39 @@ fn check_final_expr<'tcx>(
} }
}; };
if !cx.tcx.hir().attrs(expr.hir_id).is_empty() {
return;
}
let borrows = inner.map_or(false, |inner| last_statement_borrows(cx, inner)); let borrows = inner.map_or(false, |inner| last_statement_borrows(cx, inner));
if borrows { if borrows {
return; return;
} }
if ret_span.from_expansion() {
return;
}
// Returns may be used to turn an expression into a statement in rustc's AST.
// This allows the addition of attributes, like `#[allow]` (See: clippy#9361)
// `#[expect(clippy::needless_return)]` needs to be handled separatly to
// actually fullfil the expectation (clippy::#12998)
match cx.tcx.hir().attrs(expr.hir_id) {
[] => {},
[attr] => {
if matches!(Level::from_attr(attr), Some(Level::Expect(_)))
&& let metas = attr.meta_item_list()
&& let Some(lst) = metas
&& let [NestedMetaItem::MetaItem(meta_item)] = lst.as_slice()
&& let [tool, lint_name] = meta_item.path.segments.as_slice()
&& tool.ident.name == sym::clippy
&& matches!(
lint_name.ident.name.as_str(),
"needless_return" | "style" | "all" | "warnings"
)
{
// This is an expectation of the `needless_return` lint
} else {
return;
}
},
_ => return,
}
emit_return_lint(cx, ret_span, semi_spans, &replacement, expr.hir_id); emit_return_lint(cx, ret_span, semi_spans, &replacement, expr.hir_id);
}, },
@ -415,10 +448,6 @@ fn emit_return_lint(
replacement: &RetReplacement<'_>, replacement: &RetReplacement<'_>,
at: HirId, at: HirId,
) { ) {
if ret_span.from_expansion() {
return;
}
span_lint_hir_and_then( span_lint_hir_and_then(
cx, cx,
NEEDLESS_RETURN, NEEDLESS_RETURN,

View File

@ -228,12 +228,41 @@ fn needless_return_macro() -> String {
format!("Hello {}", "world!") format!("Hello {}", "world!")
} }
fn issue_9361() -> i32 { fn issue_9361(n: i32) -> i32 {
let n = 1; #[expect(clippy::arithmetic_side_effects)]
#[allow(clippy::arithmetic_side_effects)]
return n + n; return n + n;
} }
mod issue_12998 {
fn expect_lint() -> i32 {
let x = 1;
#[expect(clippy::needless_return)]
return x;
}
fn expect_group() -> i32 {
let x = 1;
#[expect(clippy::style)]
return x;
}
fn expect_all() -> i32 {
let x = 1;
#[expect(clippy::all)]
return x;
}
fn expect_warnings() -> i32 {
let x = 1;
#[expect(warnings)]
return x;
}
}
fn issue8336(x: i32) -> bool { fn issue8336(x: i32) -> bool {
if x > 0 { if x > 0 {
println!("something"); println!("something");

View File

@ -236,12 +236,41 @@ fn needless_return_macro() -> String {
return format!("Hello {}", "world!"); return format!("Hello {}", "world!");
} }
fn issue_9361() -> i32 { fn issue_9361(n: i32) -> i32 {
let n = 1; #[expect(clippy::arithmetic_side_effects)]
#[allow(clippy::arithmetic_side_effects)]
return n + n; return n + n;
} }
mod issue_12998 {
fn expect_lint() -> i32 {
let x = 1;
#[expect(clippy::needless_return)]
return x;
}
fn expect_group() -> i32 {
let x = 1;
#[expect(clippy::style)]
return x;
}
fn expect_all() -> i32 {
let x = 1;
#[expect(clippy::all)]
return x;
}
fn expect_warnings() -> i32 {
let x = 1;
#[expect(warnings)]
return x;
}
}
fn issue8336(x: i32) -> bool { fn issue8336(x: i32) -> bool {
if x > 0 { if x > 0 {
println!("something"); println!("something");

View File

@ -483,7 +483,7 @@ LL + format!("Hello {}", "world!")
| |
error: unneeded `return` statement error: unneeded `return` statement
--> tests/ui/needless_return.rs:248:9 --> tests/ui/needless_return.rs:277:9
| |
LL | return true; LL | return true;
| ^^^^^^^^^^^ | ^^^^^^^^^^^
@ -497,7 +497,7 @@ LL ~ }
| |
error: unneeded `return` statement error: unneeded `return` statement
--> tests/ui/needless_return.rs:250:9 --> tests/ui/needless_return.rs:279:9
| |
LL | return false; LL | return false;
| ^^^^^^^^^^^^ | ^^^^^^^^^^^^
@ -509,7 +509,7 @@ LL ~ }
| |
error: unneeded `return` statement error: unneeded `return` statement
--> tests/ui/needless_return.rs:257:13 --> tests/ui/needless_return.rs:286:13
| |
LL | return 10; LL | return 10;
| ^^^^^^^^^ | ^^^^^^^^^
@ -524,7 +524,7 @@ LL ~ }
| |
error: unneeded `return` statement error: unneeded `return` statement
--> tests/ui/needless_return.rs:260:13 --> tests/ui/needless_return.rs:289:13
| |
LL | return 100; LL | return 100;
| ^^^^^^^^^^ | ^^^^^^^^^^
@ -537,7 +537,7 @@ LL ~ }
| |
error: unneeded `return` statement error: unneeded `return` statement
--> tests/ui/needless_return.rs:268:9 --> tests/ui/needless_return.rs:297:9
| |
LL | return 0; LL | return 0;
| ^^^^^^^^ | ^^^^^^^^
@ -549,7 +549,7 @@ LL ~ }
| |
error: unneeded `return` statement error: unneeded `return` statement
--> tests/ui/needless_return.rs:275:13 --> tests/ui/needless_return.rs:304:13
| |
LL | return *(x as *const isize); LL | return *(x as *const isize);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -564,7 +564,7 @@ LL ~ }
| |
error: unneeded `return` statement error: unneeded `return` statement
--> tests/ui/needless_return.rs:277:13 --> tests/ui/needless_return.rs:306:13
| |
LL | return !*(x as *const isize); LL | return !*(x as *const isize);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -577,7 +577,7 @@ LL ~ }
| |
error: unneeded `return` statement error: unneeded `return` statement
--> tests/ui/needless_return.rs:284:20 --> tests/ui/needless_return.rs:313:20
| |
LL | let _ = 42; LL | let _ = 42;
| ____________________^ | ____________________^
@ -594,7 +594,7 @@ LL + let _ = 42;
| |
error: unneeded `return` statement error: unneeded `return` statement
--> tests/ui/needless_return.rs:291:20 --> tests/ui/needless_return.rs:320:20
| |
LL | let _ = 42; return; LL | let _ = 42; return;
| ^^^^^^^ | ^^^^^^^
@ -606,7 +606,7 @@ LL + let _ = 42;
| |
error: unneeded `return` statement error: unneeded `return` statement
--> tests/ui/needless_return.rs:303:9 --> tests/ui/needless_return.rs:332:9
| |
LL | return Ok(format!("ok!")); LL | return Ok(format!("ok!"));
| ^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^
@ -618,7 +618,7 @@ LL + Ok(format!("ok!"))
| |
error: unneeded `return` statement error: unneeded `return` statement
--> tests/ui/needless_return.rs:305:9 --> tests/ui/needless_return.rs:334:9
| |
LL | return Err(format!("err!")); LL | return Err(format!("err!"));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -630,7 +630,7 @@ LL + Err(format!("err!"))
| |
error: unneeded `return` statement error: unneeded `return` statement
--> tests/ui/needless_return.rs:311:9 --> tests/ui/needless_return.rs:340:9
| |
LL | return if true { 1 } else { 2 }; LL | return if true { 1 } else { 2 };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -642,7 +642,7 @@ LL + if true { 1 } else { 2 }
| |
error: unneeded `return` statement error: unneeded `return` statement
--> tests/ui/needless_return.rs:315:9 --> tests/ui/needless_return.rs:344:9
| |
LL | return if b1 { 0 } else { 1 } | if b2 { 2 } else { 3 } | if b3 { 4 } else { 5 }; LL | return if b1 { 0 } else { 1 } | if b2 { 2 } else { 3 } | if b3 { 4 } else { 5 };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -654,7 +654,7 @@ LL + (if b1 { 0 } else { 1 } | if b2 { 2 } else { 3 } | if b3 { 4 } else
| |
error: unneeded `return` statement error: unneeded `return` statement
--> tests/ui/needless_return.rs:336:5 --> tests/ui/needless_return.rs:365:5
| |
LL | return { "a".to_string() } + "b" + { "c" }; LL | return { "a".to_string() } + "b" + { "c" };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^