suggest adding parentheses when linting [let_and_return
] and [needless_return
]
This commit is contained in:
parent
05c4053628
commit
03306b6ab6
@ -2,7 +2,9 @@
|
||||
use clippy_utils::source::{snippet_with_applicability, snippet_with_context};
|
||||
use clippy_utils::sugg::has_enclosing_paren;
|
||||
use clippy_utils::ty::{implements_trait, is_manually_drop, peel_mid_ty_refs};
|
||||
use clippy_utils::{expr_use_ctxt, get_parent_expr, is_lint_allowed, path_to_local, DefinedTy, ExprUseNode};
|
||||
use clippy_utils::{
|
||||
expr_use_ctxt, get_parent_expr, is_block_like, is_lint_allowed, path_to_local, DefinedTy, ExprUseNode,
|
||||
};
|
||||
use core::mem;
|
||||
use rustc_ast::util::parser::{PREC_POSTFIX, PREC_PREFIX};
|
||||
use rustc_data_structures::fx::FxIndexMap;
|
||||
@ -1038,14 +1040,8 @@ fn report<'tcx>(
|
||||
);
|
||||
},
|
||||
State::ExplicitDeref { mutability } => {
|
||||
if matches!(
|
||||
expr.kind,
|
||||
ExprKind::Block(..)
|
||||
| ExprKind::ConstBlock(_)
|
||||
| ExprKind::If(..)
|
||||
| ExprKind::Loop(..)
|
||||
| ExprKind::Match(..)
|
||||
) && let ty::Ref(_, ty, _) = data.adjusted_ty.kind()
|
||||
if is_block_like(expr)
|
||||
&& let ty::Ref(_, ty, _) = data.adjusted_ty.kind()
|
||||
&& ty.is_sized(cx.tcx, cx.param_env)
|
||||
{
|
||||
// Rustc bug: auto deref doesn't work on block expression when targeting sized types.
|
||||
|
@ -6,8 +6,8 @@
|
||||
use clippy_utils::source::snippet_with_applicability;
|
||||
use clippy_utils::sugg::Sugg;
|
||||
use clippy_utils::{
|
||||
higher, is_else_clause, is_expn_of, is_parent_stmt, peel_blocks, peel_blocks_with_stmt, span_extract_comment,
|
||||
SpanlessEq,
|
||||
higher, is_block_like, is_else_clause, is_expn_of, is_parent_stmt, peel_blocks, peel_blocks_with_stmt,
|
||||
span_extract_comment, SpanlessEq,
|
||||
};
|
||||
use rustc_ast::ast::LitKind;
|
||||
use rustc_errors::Applicability;
|
||||
@ -121,14 +121,7 @@ fn condition_needs_parentheses(e: &Expr<'_>) -> bool {
|
||||
| ExprKind::Type(i, _)
|
||||
| ExprKind::Index(i, _, _) = inner.kind
|
||||
{
|
||||
if matches!(
|
||||
i.kind,
|
||||
ExprKind::Block(..)
|
||||
| ExprKind::ConstBlock(..)
|
||||
| ExprKind::If(..)
|
||||
| ExprKind::Loop(..)
|
||||
| ExprKind::Match(..)
|
||||
) {
|
||||
if is_block_like(i) {
|
||||
return true;
|
||||
}
|
||||
inner = i;
|
||||
|
@ -3,8 +3,8 @@
|
||||
use clippy_utils::sugg::has_enclosing_paren;
|
||||
use clippy_utils::visitors::{for_each_expr_with_closures, Descend};
|
||||
use clippy_utils::{
|
||||
fn_def_id, is_from_proc_macro, is_inside_let_else, is_res_lang_ctor, path_res, path_to_local_id, span_contains_cfg,
|
||||
span_find_starting_semi,
|
||||
binary_expr_needs_parentheses, fn_def_id, is_from_proc_macro, is_inside_let_else, is_res_lang_ctor, path_res,
|
||||
path_to_local_id, span_contains_cfg, span_find_starting_semi,
|
||||
};
|
||||
use core::ops::ControlFlow;
|
||||
use rustc_errors::Applicability;
|
||||
@ -129,7 +129,7 @@ enum RetReplacement<'tcx> {
|
||||
Empty,
|
||||
Block,
|
||||
Unit,
|
||||
IfSequence(Cow<'tcx, str>, Applicability),
|
||||
NeedsPar(Cow<'tcx, str>, Applicability),
|
||||
Expr(Cow<'tcx, str>, Applicability),
|
||||
}
|
||||
|
||||
@ -139,13 +139,13 @@ fn sugg_help(&self) -> &'static str {
|
||||
Self::Empty | Self::Expr(..) => "remove `return`",
|
||||
Self::Block => "replace `return` with an empty block",
|
||||
Self::Unit => "replace `return` with a unit value",
|
||||
Self::IfSequence(..) => "remove `return` and wrap the sequence with parentheses",
|
||||
Self::NeedsPar(..) => "remove `return` and wrap the sequence with parentheses",
|
||||
}
|
||||
}
|
||||
|
||||
fn applicability(&self) -> Applicability {
|
||||
match self {
|
||||
Self::Expr(_, ap) | Self::IfSequence(_, ap) => *ap,
|
||||
Self::Expr(_, ap) | Self::NeedsPar(_, ap) => *ap,
|
||||
_ => Applicability::MachineApplicable,
|
||||
}
|
||||
}
|
||||
@ -157,7 +157,7 @@ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
Self::Empty => write!(f, ""),
|
||||
Self::Block => write!(f, "{{}}"),
|
||||
Self::Unit => write!(f, "()"),
|
||||
Self::IfSequence(inner, _) => write!(f, "({inner})"),
|
||||
Self::NeedsPar(inner, _) => write!(f, "({inner})"),
|
||||
Self::Expr(inner, _) => write!(f, "{inner}"),
|
||||
}
|
||||
}
|
||||
@ -244,7 +244,11 @@ fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'_>) {
|
||||
err.span_label(local.span, "unnecessary `let` binding");
|
||||
|
||||
if let Some(mut snippet) = snippet_opt(cx, initexpr.span) {
|
||||
if !cx.typeck_results().expr_adjustments(retexpr).is_empty() {
|
||||
if binary_expr_needs_parentheses(initexpr) {
|
||||
if !has_enclosing_paren(&snippet) {
|
||||
snippet = format!("({snippet})");
|
||||
}
|
||||
} else if !cx.typeck_results().expr_adjustments(retexpr).is_empty() {
|
||||
if !has_enclosing_paren(&snippet) {
|
||||
snippet = format!("({snippet})");
|
||||
}
|
||||
@ -349,8 +353,8 @@ fn check_final_expr<'tcx>(
|
||||
|
||||
let mut applicability = Applicability::MachineApplicable;
|
||||
let (snippet, _) = snippet_with_context(cx, inner_expr.span, ret_span.ctxt(), "..", &mut applicability);
|
||||
if expr_contains_conjunctive_ifs(inner_expr) {
|
||||
RetReplacement::IfSequence(snippet, applicability)
|
||||
if binary_expr_needs_parentheses(inner_expr) {
|
||||
RetReplacement::NeedsPar(snippet, applicability)
|
||||
} else {
|
||||
RetReplacement::Expr(snippet, applicability)
|
||||
}
|
||||
@ -404,18 +408,6 @@ fn check_final_expr<'tcx>(
|
||||
}
|
||||
}
|
||||
|
||||
fn expr_contains_conjunctive_ifs<'tcx>(expr: &'tcx Expr<'tcx>) -> bool {
|
||||
fn contains_if(expr: &Expr<'_>, on_if: bool) -> bool {
|
||||
match expr.kind {
|
||||
ExprKind::If(..) => on_if,
|
||||
ExprKind::Binary(_, left, right) => contains_if(left, true) || contains_if(right, true),
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
contains_if(expr, false)
|
||||
}
|
||||
|
||||
fn emit_return_lint(
|
||||
cx: &LateContext<'_>,
|
||||
ret_span: Span,
|
||||
|
@ -3370,3 +3370,25 @@ pub fn is_parent_stmt(cx: &LateContext<'_>, id: HirId) -> bool {
|
||||
Node::Stmt(..) | Node::Block(Block { stmts: &[], .. })
|
||||
)
|
||||
}
|
||||
|
||||
/// Returns true if the given `expr` is a block or resembled as a block,
|
||||
/// such as `if`, `loop`, `match` expressions etc.
|
||||
pub fn is_block_like(expr: &Expr<'_>) -> bool {
|
||||
matches!(
|
||||
expr.kind,
|
||||
ExprKind::Block(..) | ExprKind::ConstBlock(..) | ExprKind::If(..) | ExprKind::Loop(..) | ExprKind::Match(..)
|
||||
)
|
||||
}
|
||||
|
||||
/// Returns true if the given `expr` is binary expression that needs to be wrapped in parentheses.
|
||||
pub fn binary_expr_needs_parentheses(expr: &Expr<'_>) -> bool {
|
||||
fn contains_block(expr: &Expr<'_>, is_operand: bool) -> bool {
|
||||
match expr.kind {
|
||||
ExprKind::Binary(_, lhs, _) => contains_block(lhs, true),
|
||||
_ if is_block_like(expr) => is_operand,
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
contains_block(expr, false)
|
||||
}
|
||||
|
@ -210,4 +210,38 @@ fn issue9150() -> usize {
|
||||
x
|
||||
}
|
||||
|
||||
fn issue12801() {
|
||||
fn left_is_if() -> String {
|
||||
|
||||
(if true { "a".to_string() } else { "b".to_string() } + "c")
|
||||
//~^ ERROR: returning the result of a `let` binding from a block
|
||||
}
|
||||
|
||||
fn no_par_needed() -> String {
|
||||
|
||||
"c".to_string() + if true { "a" } else { "b" }
|
||||
//~^ ERROR: returning the result of a `let` binding from a block
|
||||
}
|
||||
|
||||
fn conjunctive_blocks() -> String {
|
||||
|
||||
({ "a".to_string() } + "b" + { "c" } + "d")
|
||||
//~^ ERROR: returning the result of a `let` binding from a block
|
||||
}
|
||||
|
||||
#[allow(clippy::overly_complex_bool_expr)]
|
||||
fn other_ops() {
|
||||
let _ = || {
|
||||
|
||||
(if true { 2 } else { 3 } << 4)
|
||||
//~^ ERROR: returning the result of a `let` binding from a block
|
||||
};
|
||||
let _ = || {
|
||||
|
||||
({ true } || { false } && { 2 <= 3 })
|
||||
//~^ ERROR: returning the result of a `let` binding from a block
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
|
@ -210,4 +210,38 @@ fn issue9150() -> usize {
|
||||
x
|
||||
}
|
||||
|
||||
fn issue12801() {
|
||||
fn left_is_if() -> String {
|
||||
let s = if true { "a".to_string() } else { "b".to_string() } + "c";
|
||||
s
|
||||
//~^ ERROR: returning the result of a `let` binding from a block
|
||||
}
|
||||
|
||||
fn no_par_needed() -> String {
|
||||
let s = "c".to_string() + if true { "a" } else { "b" };
|
||||
s
|
||||
//~^ ERROR: returning the result of a `let` binding from a block
|
||||
}
|
||||
|
||||
fn conjunctive_blocks() -> String {
|
||||
let s = { "a".to_string() } + "b" + { "c" } + "d";
|
||||
s
|
||||
//~^ ERROR: returning the result of a `let` binding from a block
|
||||
}
|
||||
|
||||
#[allow(clippy::overly_complex_bool_expr)]
|
||||
fn other_ops() {
|
||||
let _ = || {
|
||||
let s = if true { 2 } else { 3 } << 4;
|
||||
s
|
||||
//~^ ERROR: returning the result of a `let` binding from a block
|
||||
};
|
||||
let _ = || {
|
||||
let s = { true } || { false } && { 2 <= 3 };
|
||||
s
|
||||
//~^ ERROR: returning the result of a `let` binding from a block
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
|
@ -78,5 +78,75 @@ LL + E::B(x) => x,
|
||||
LL + }) as _
|
||||
|
|
||||
|
||||
error: aborting due to 5 previous errors
|
||||
error: returning the result of a `let` binding from a block
|
||||
--> tests/ui/let_and_return.rs:216:9
|
||||
|
|
||||
LL | let s = if true { "a".to_string() } else { "b".to_string() } + "c";
|
||||
| ------------------------------------------------------------------- unnecessary `let` binding
|
||||
LL | s
|
||||
| ^
|
||||
|
|
||||
help: return the expression directly
|
||||
|
|
||||
LL ~
|
||||
LL ~ (if true { "a".to_string() } else { "b".to_string() } + "c")
|
||||
|
|
||||
|
||||
error: returning the result of a `let` binding from a block
|
||||
--> tests/ui/let_and_return.rs:222:9
|
||||
|
|
||||
LL | let s = "c".to_string() + if true { "a" } else { "b" };
|
||||
| ------------------------------------------------------- unnecessary `let` binding
|
||||
LL | s
|
||||
| ^
|
||||
|
|
||||
help: return the expression directly
|
||||
|
|
||||
LL ~
|
||||
LL ~ "c".to_string() + if true { "a" } else { "b" }
|
||||
|
|
||||
|
||||
error: returning the result of a `let` binding from a block
|
||||
--> tests/ui/let_and_return.rs:228:9
|
||||
|
|
||||
LL | let s = { "a".to_string() } + "b" + { "c" } + "d";
|
||||
| -------------------------------------------------- unnecessary `let` binding
|
||||
LL | s
|
||||
| ^
|
||||
|
|
||||
help: return the expression directly
|
||||
|
|
||||
LL ~
|
||||
LL ~ ({ "a".to_string() } + "b" + { "c" } + "d")
|
||||
|
|
||||
|
||||
error: returning the result of a `let` binding from a block
|
||||
--> tests/ui/let_and_return.rs:236:13
|
||||
|
|
||||
LL | let s = if true { 2 } else { 3 } << 4;
|
||||
| -------------------------------------- unnecessary `let` binding
|
||||
LL | s
|
||||
| ^
|
||||
|
|
||||
help: return the expression directly
|
||||
|
|
||||
LL ~
|
||||
LL ~ (if true { 2 } else { 3 } << 4)
|
||||
|
|
||||
|
||||
error: returning the result of a `let` binding from a block
|
||||
--> tests/ui/let_and_return.rs:241:13
|
||||
|
|
||||
LL | let s = { true } || { false } && { 2 <= 3 };
|
||||
| -------------------------------------------- unnecessary `let` binding
|
||||
LL | s
|
||||
| ^
|
||||
|
|
||||
help: return the expression directly
|
||||
|
|
||||
LL ~
|
||||
LL ~ ({ true } || { false } && { 2 <= 3 })
|
||||
|
|
||||
|
||||
error: aborting due to 10 previous errors
|
||||
|
||||
|
@ -323,4 +323,8 @@ fn allow_works() -> i32 {
|
||||
}
|
||||
}
|
||||
|
||||
fn conjunctive_blocks() -> String {
|
||||
({ "a".to_string() } + "b" + { "c" })
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
|
@ -333,4 +333,8 @@ fn allow_works() -> i32 {
|
||||
}
|
||||
}
|
||||
|
||||
fn conjunctive_blocks() -> String {
|
||||
return { "a".to_string() } + "b" + { "c" };
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
|
@ -653,5 +653,17 @@ LL - return if b1 { 0 } else { 1 } | if b2 { 2 } else { 3 } | if b3 { 4
|
||||
LL + (if b1 { 0 } else { 1 } | if b2 { 2 } else { 3 } | if b3 { 4 } else { 5 })
|
||||
|
|
||||
|
||||
error: aborting due to 52 previous errors
|
||||
error: unneeded `return` statement
|
||||
--> tests/ui/needless_return.rs:337:5
|
||||
|
|
||||
LL | return { "a".to_string() } + "b" + { "c" };
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: remove `return` and wrap the sequence with parentheses
|
||||
|
|
||||
LL - return { "a".to_string() } + "b" + { "c" };
|
||||
LL + ({ "a".to_string() } + "b" + { "c" })
|
||||
|
|
||||
|
||||
error: aborting due to 53 previous errors
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user