Fix match_single_binding for assign expressions
This commit is contained in:
Serial
parent
d422baa30c
commit
554dc41cea
|
|
@ -1,15 +1,22 @@
|
|||
use clippy_utils::diagnostics::span_lint_and_sugg;
|
||||
use clippy_utils::source::{indent_of, snippet_block, snippet_with_applicability};
|
||||
use clippy_utils::macros::HirNode;
|
||||
use clippy_utils::source::{indent_of, snippet, snippet_block, snippet_with_applicability};
|
||||
use clippy_utils::sugg::Sugg;
|
||||
use clippy_utils::{get_parent_expr, is_refutable, peel_blocks};
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::{Arm, Expr, ExprKind, Local, Node, PatKind};
|
||||
use rustc_hir::{Arm, Expr, ExprKind, Node, PatKind};
|
||||
use rustc_lint::LateContext;
|
||||
use rustc_span::Span;
|
||||
|
||||
use super::MATCH_SINGLE_BINDING;
|
||||
|
||||
enum AssignmentExpr {
|
||||
Assign { span: Span, match_span: Span },
|
||||
Local { span: Span, pat_span: Span },
|
||||
}
|
||||
|
||||
#[expect(clippy::too_many_lines)]
|
||||
pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], expr: &Expr<'_>) {
|
||||
pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], expr: &Expr<'a>) {
|
||||
if expr.span.from_expansion() || arms.len() != 1 || is_refutable(cx, arms[0].pat) {
|
||||
return;
|
||||
}
|
||||
|
|
@ -42,61 +49,59 @@ pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], e
|
|||
let mut applicability = Applicability::MaybeIncorrect;
|
||||
match arms[0].pat.kind {
|
||||
PatKind::Binding(..) | PatKind::Tuple(_, _) | PatKind::Struct(..) => {
|
||||
// If this match is in a local (`let`) stmt
|
||||
let (target_span, sugg) = if let Some(parent_let_node) = opt_parent_let(cx, ex) {
|
||||
(
|
||||
parent_let_node.span,
|
||||
let (target_span, sugg) = match opt_parent_assign_span(cx, ex) {
|
||||
Some(AssignmentExpr::Assign { span, match_span }) => {
|
||||
let sugg = sugg_with_curlies(
|
||||
cx,
|
||||
(ex, expr),
|
||||
(bind_names, matched_vars),
|
||||
&*snippet_body,
|
||||
&mut applicability,
|
||||
Some(span),
|
||||
);
|
||||
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
MATCH_SINGLE_BINDING,
|
||||
span.to(match_span),
|
||||
"this assignment could be simplified",
|
||||
"consider removing the `match` expression",
|
||||
sugg,
|
||||
applicability,
|
||||
);
|
||||
|
||||
return;
|
||||
},
|
||||
Some(AssignmentExpr::Local { span, pat_span }) => (
|
||||
span,
|
||||
format!(
|
||||
"let {} = {};\n{}let {} = {};",
|
||||
snippet_with_applicability(cx, bind_names, "..", &mut applicability),
|
||||
snippet_with_applicability(cx, matched_vars, "..", &mut applicability),
|
||||
" ".repeat(indent_of(cx, expr.span).unwrap_or(0)),
|
||||
snippet_with_applicability(cx, parent_let_node.pat.span, "..", &mut applicability),
|
||||
snippet_with_applicability(cx, pat_span, "..", &mut applicability),
|
||||
snippet_body
|
||||
),
|
||||
)
|
||||
} else {
|
||||
// If we are in closure, we need curly braces around suggestion
|
||||
let mut indent = " ".repeat(indent_of(cx, ex.span).unwrap_or(0));
|
||||
let (mut cbrace_start, mut cbrace_end) = ("".to_string(), "".to_string());
|
||||
if let Some(parent_expr) = get_parent_expr(cx, expr) {
|
||||
if let ExprKind::Closure(..) = parent_expr.kind {
|
||||
cbrace_end = format!("\n{}}}", indent);
|
||||
// Fix body indent due to the closure
|
||||
indent = " ".repeat(indent_of(cx, bind_names).unwrap_or(0));
|
||||
cbrace_start = format!("{{\n{}", indent);
|
||||
}
|
||||
}
|
||||
// If the parent is already an arm, and the body is another match statement,
|
||||
// we need curly braces around suggestion
|
||||
let parent_node_id = cx.tcx.hir().get_parent_node(expr.hir_id);
|
||||
if let Node::Arm(arm) = &cx.tcx.hir().get(parent_node_id) {
|
||||
if let ExprKind::Match(..) = arm.body.kind {
|
||||
cbrace_end = format!("\n{}}}", indent);
|
||||
// Fix body indent due to the match
|
||||
indent = " ".repeat(indent_of(cx, bind_names).unwrap_or(0));
|
||||
cbrace_start = format!("{{\n{}", indent);
|
||||
}
|
||||
}
|
||||
(
|
||||
expr.span,
|
||||
format!(
|
||||
"{}let {} = {};\n{}{}{}",
|
||||
cbrace_start,
|
||||
snippet_with_applicability(cx, bind_names, "..", &mut applicability),
|
||||
snippet_with_applicability(cx, matched_vars, "..", &mut applicability),
|
||||
indent,
|
||||
snippet_body,
|
||||
cbrace_end
|
||||
),
|
||||
)
|
||||
),
|
||||
None => {
|
||||
let sugg = sugg_with_curlies(
|
||||
cx,
|
||||
(ex, expr),
|
||||
(bind_names, matched_vars),
|
||||
&*snippet_body,
|
||||
&mut applicability,
|
||||
None,
|
||||
);
|
||||
(expr.span, sugg)
|
||||
},
|
||||
};
|
||||
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
MATCH_SINGLE_BINDING,
|
||||
target_span,
|
||||
"this match could be written as a `let` statement",
|
||||
"consider using `let` statement",
|
||||
"consider using a `let` statement",
|
||||
sugg,
|
||||
applicability,
|
||||
);
|
||||
|
|
@ -110,6 +115,7 @@ pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], e
|
|||
indent,
|
||||
snippet_body
|
||||
);
|
||||
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
MATCH_SINGLE_BINDING,
|
||||
|
|
@ -135,15 +141,76 @@ pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], e
|
|||
}
|
||||
}
|
||||
|
||||
/// Returns true if the `ex` match expression is in a local (`let`) statement
|
||||
fn opt_parent_let<'a>(cx: &LateContext<'a>, ex: &Expr<'a>) -> Option<&'a Local<'a>> {
|
||||
/// Returns true if the `ex` match expression is in a local (`let`) or assign expression
|
||||
fn opt_parent_assign_span<'a>(cx: &LateContext<'a>, ex: &Expr<'a>) -> Option<AssignmentExpr> {
|
||||
let map = &cx.tcx.hir();
|
||||
if_chain! {
|
||||
if let Some(Node::Expr(parent_arm_expr)) = map.find(map.get_parent_node(ex.hir_id));
|
||||
if let Some(Node::Local(parent_let_expr)) = map.find(map.get_parent_node(parent_arm_expr.hir_id));
|
||||
then {
|
||||
return Some(parent_let_expr);
|
||||
}
|
||||
|
||||
if let Some(Node::Expr(parent_arm_expr)) = map.find(map.get_parent_node(ex.hir_id)) {
|
||||
return match map.find(map.get_parent_node(parent_arm_expr.hir_id)) {
|
||||
Some(Node::Local(parent_let_expr)) => Some(AssignmentExpr::Local {
|
||||
span: parent_let_expr.span,
|
||||
pat_span: parent_let_expr.pat.span(),
|
||||
}),
|
||||
Some(Node::Expr(Expr {
|
||||
kind: ExprKind::Assign(parent_assign_expr, match_expr, _),
|
||||
..
|
||||
})) => Some(AssignmentExpr::Assign {
|
||||
span: parent_assign_expr.span,
|
||||
match_span: match_expr.span,
|
||||
}),
|
||||
_ => None,
|
||||
};
|
||||
}
|
||||
|
||||
None
|
||||
}
|
||||
|
||||
fn sugg_with_curlies<'a>(
|
||||
cx: &LateContext<'a>,
|
||||
(ex, match_expr): (&Expr<'a>, &Expr<'a>),
|
||||
(bind_names, matched_vars): (Span, Span),
|
||||
snippet_body: &str,
|
||||
applicability: &mut Applicability,
|
||||
assignment: Option<Span>,
|
||||
) -> String {
|
||||
let mut indent = " ".repeat(indent_of(cx, ex.span).unwrap_or(0));
|
||||
|
||||
let (mut cbrace_start, mut cbrace_end) = (String::new(), String::new());
|
||||
if let Some(parent_expr) = get_parent_expr(cx, match_expr) {
|
||||
if let ExprKind::Closure(..) = parent_expr.kind {
|
||||
cbrace_end = format!("\n{}}}", indent);
|
||||
// Fix body indent due to the closure
|
||||
indent = " ".repeat(indent_of(cx, bind_names).unwrap_or(0));
|
||||
cbrace_start = format!("{{\n{}", indent);
|
||||
}
|
||||
}
|
||||
|
||||
// If the parent is already an arm, and the body is another match statement,
|
||||
// we need curly braces around suggestion
|
||||
let parent_node_id = cx.tcx.hir().get_parent_node(match_expr.hir_id);
|
||||
if let Node::Arm(arm) = &cx.tcx.hir().get(parent_node_id) {
|
||||
if let ExprKind::Match(..) = arm.body.kind {
|
||||
cbrace_end = format!("\n{}}}", indent);
|
||||
// Fix body indent due to the match
|
||||
indent = " ".repeat(indent_of(cx, bind_names).unwrap_or(0));
|
||||
cbrace_start = format!("{{\n{}", indent);
|
||||
}
|
||||
}
|
||||
|
||||
let assignment_str = assignment.map_or_else(String::new, |span| {
|
||||
let mut s = snippet(cx, span, "..").to_string();
|
||||
s.push_str(" = ");
|
||||
s
|
||||
});
|
||||
|
||||
format!(
|
||||
"{}let {} = {};\n{}{}{}{}",
|
||||
cbrace_start,
|
||||
snippet_with_applicability(cx, bind_names, "..", applicability),
|
||||
snippet_with_applicability(cx, matched_vars, "..", applicability),
|
||||
indent,
|
||||
assignment_str,
|
||||
snippet_body,
|
||||
cbrace_end
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -111,3 +111,16 @@ fn main() {
|
|||
let x = 1;
|
||||
println!("Not an array index start");
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
fn issue_8723() {
|
||||
let (mut val, idx) = ("a b", 1);
|
||||
|
||||
let (pre, suf) = val.split_at(idx);
|
||||
val = {
|
||||
println!("{}", pre);
|
||||
suf
|
||||
};
|
||||
|
||||
let _ = val;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -126,3 +126,17 @@ fn main() {
|
|||
_ => println!("Not an array index start"),
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
fn issue_8723() {
|
||||
let (mut val, idx) = ("a b", 1);
|
||||
|
||||
val = match val.split_at(idx) {
|
||||
(pre, suf) => {
|
||||
println!("{}", pre);
|
||||
suf
|
||||
},
|
||||
};
|
||||
|
||||
let _ = val;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@ LL | | }
|
|||
| |_____^
|
||||
|
|
||||
= note: `-D clippy::match-single-binding` implied by `-D warnings`
|
||||
help: consider using `let` statement
|
||||
help: consider using a `let` statement
|
||||
|
|
||||
LL ~ let (x, y, z) = (a, b, c);
|
||||
LL + {
|
||||
|
|
@ -25,7 +25,7 @@ LL | | (x, y, z) => println!("{} {} {}", x, y, z),
|
|||
LL | | }
|
||||
| |_____^
|
||||
|
|
||||
help: consider using `let` statement
|
||||
help: consider using a `let` statement
|
||||
|
|
||||
LL ~ let (x, y, z) = (a, b, c);
|
||||
LL + println!("{} {} {}", x, y, z);
|
||||
|
|
@ -88,7 +88,7 @@ LL | | Point { x, y } => println!("Coords: ({}, {})", x, y),
|
|||
LL | | }
|
||||
| |_____^
|
||||
|
|
||||
help: consider using `let` statement
|
||||
help: consider using a `let` statement
|
||||
|
|
||||
LL ~ let Point { x, y } = p;
|
||||
LL + println!("Coords: ({}, {})", x, y);
|
||||
|
|
@ -102,7 +102,7 @@ LL | | Point { x: x1, y: y1 } => println!("Coords: ({}, {})", x1, y1),
|
|||
LL | | }
|
||||
| |_____^
|
||||
|
|
||||
help: consider using `let` statement
|
||||
help: consider using a `let` statement
|
||||
|
|
||||
LL ~ let Point { x: x1, y: y1 } = p;
|
||||
LL + println!("Coords: ({}, {})", x1, y1);
|
||||
|
|
@ -116,7 +116,7 @@ LL | | ref r => println!("Got a reference to {}", r),
|
|||
LL | | }
|
||||
| |_____^
|
||||
|
|
||||
help: consider using `let` statement
|
||||
help: consider using a `let` statement
|
||||
|
|
||||
LL ~ let ref r = x;
|
||||
LL + println!("Got a reference to {}", r);
|
||||
|
|
@ -130,7 +130,7 @@ LL | | ref mut mr => println!("Got a mutable reference to {}", mr),
|
|||
LL | | }
|
||||
| |_____^
|
||||
|
|
||||
help: consider using `let` statement
|
||||
help: consider using a `let` statement
|
||||
|
|
||||
LL ~ let ref mut mr = x;
|
||||
LL + println!("Got a mutable reference to {}", mr);
|
||||
|
|
@ -144,7 +144,7 @@ LL | | Point { x, y } => x * y,
|
|||
LL | | };
|
||||
| |______^
|
||||
|
|
||||
help: consider using `let` statement
|
||||
help: consider using a `let` statement
|
||||
|
|
||||
LL ~ let Point { x, y } = coords();
|
||||
LL + let product = x * y;
|
||||
|
|
@ -159,7 +159,7 @@ LL | | unwrapped => unwrapped,
|
|||
LL | | })
|
||||
| |_________^
|
||||
|
|
||||
help: consider using `let` statement
|
||||
help: consider using a `let` statement
|
||||
|
|
||||
LL ~ .map(|i| {
|
||||
LL + let unwrapped = i.unwrap();
|
||||
|
|
@ -176,5 +176,25 @@ LL | | _ => println!("Not an array index start"),
|
|||
LL | | }
|
||||
| |_____^ help: consider using the match body instead: `println!("Not an array index start");`
|
||||
|
||||
error: aborting due to 12 previous errors
|
||||
error: this assignment could be simplified
|
||||
--> $DIR/match_single_binding.rs:134:5
|
||||
|
|
||||
LL | / val = match val.split_at(idx) {
|
||||
LL | | (pre, suf) => {
|
||||
LL | | println!("{}", pre);
|
||||
LL | | suf
|
||||
LL | | },
|
||||
LL | | };
|
||||
| |_____^
|
||||
|
|
||||
help: consider removing the `match` expression
|
||||
|
|
||||
LL ~ let (pre, suf) = val.split_at(idx);
|
||||
LL + val = {
|
||||
LL + println!("{}", pre);
|
||||
LL + suf
|
||||
LL ~ };
|
||||
|
|
||||
|
||||
error: aborting due to 13 previous errors
|
||||
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ LL | | },
|
|||
| |_____________^
|
||||
|
|
||||
= note: `-D clippy::match-single-binding` implied by `-D warnings`
|
||||
help: consider using `let` statement
|
||||
help: consider using a `let` statement
|
||||
|
|
||||
LL ~ Some((iter, _item)) => {
|
||||
LL + let (min, max) = iter.size_hint();
|
||||
|
|
@ -24,7 +24,7 @@ LL | | (a, b) => println!("a {:?} and b {:?}", a, b),
|
|||
LL | | }
|
||||
| |_____________^
|
||||
|
|
||||
help: consider using `let` statement
|
||||
help: consider using a `let` statement
|
||||
|
|
||||
LL ~ let (a, b) = get_tup();
|
||||
LL + println!("a {:?} and b {:?}", a, b);
|
||||
|
|
|
|||
Loading…
Reference in a new issue