From 179c037643fda44f7874816c0030ad83798bdad3 Mon Sep 17 00:00:00 2001 From: DevAccentor Date: Sat, 4 Jun 2022 16:48:35 +0200 Subject: [PATCH] improve almost swap to look for let statement --- clippy_lints/src/swap.rs | 74 ++++++++++++++++++++++++++++++++++ tests/ui/almost_swapped.rs | 32 +++++++++++++++ tests/ui/almost_swapped.stderr | 30 ++++++++++++++ 3 files changed, 136 insertions(+) create mode 100644 tests/ui/almost_swapped.rs create mode 100644 tests/ui/almost_swapped.stderr diff --git a/clippy_lints/src/swap.rs b/clippy_lints/src/swap.rs index 17e9cc5f6b7..5c408a73a55 100644 --- a/clippy_lints/src/swap.rs +++ b/clippy_lints/src/swap.rs @@ -172,6 +172,7 @@ fn check_manual_swap(cx: &LateContext<'_>, block: &Block<'_>) { } } +#[allow(clippy::too_many_lines)] /// Implementation of the `ALMOST_SWAPPED` lint. fn check_suspicious_swap(cx: &LateContext<'_>, block: &Block<'_>) { for w in block.stmts.windows(2) { @@ -220,6 +221,79 @@ fn check_suspicious_swap(cx: &LateContext<'_>, block: &Block<'_>) { }); } } + + let lint_almost_swapped_note = |span, what: String, sugg, lhs, rhs| { + span_lint_and_then( + cx, + ALMOST_SWAPPED, + span, + &format!("this looks like you are trying to swap{}", what), + |diag| { + if !what.is_empty() { + diag.note(&format!( + "maybe you could use `{sugg}::mem::swap({lhs}, {rhs})` or `{sugg}::mem::replace`?" + )); + } + }, + ); + }; + + if let StmtKind::Local(first) = w[0].kind + && let StmtKind::Local(second) = w[1].kind + && first.span.ctxt() == second.span.ctxt() + && let Some(rhs0) = first.init + && let Some(rhs1) = second.init + && let ExprKind::Path(QPath::Resolved(None, path_l)) = rhs0.kind + && let ExprKind::Path(QPath::Resolved(None, path_r)) = rhs1.kind + && let PatKind::Binding(_,_, ident_l,_) = first.pat.kind + && let PatKind::Binding(_,_, ident_r,_) = second.pat.kind + && ident_l.name.as_str() == path_r.segments.iter().map(|el| el.ident.to_string()).collect::>().join("::") + && ident_r.name.as_str() == path_l.segments.iter().map(|el| el.ident.to_string()).collect::>().join("::") + { + let rhs0 = Sugg::hir_opt(cx, rhs0); + let (what, lhs, rhs) = if let Some(second) = rhs0 { + ( + format!(" `{}` and `{}`", ident_l, second), + format!("&mut {}", ident_l), + second.mut_addr().to_string(), + ) + } else { + (String::new(), String::new(), String::new()) + }; + let span = first.span.to(second.span); + let Some(sugg) = std_or_core(cx) else { return }; + + lint_almost_swapped_note(span, what, sugg, lhs, rhs); + } + + if let StmtKind::Local(first) = w[0].kind + && let StmtKind::Semi(second) = w[1].kind + && first.span.ctxt() == second.span.ctxt() + && let Some(rhs0) = first.init + && let ExprKind::Path(QPath::Resolved(None, path_l)) = rhs0.kind + && let PatKind::Binding(_,_, ident_l,_) = first.pat.kind + && let ExprKind::Assign(lhs1, rhs1, _) = second.kind + && let ExprKind::Path(QPath::Resolved(None, lhs1_path)) = lhs1.kind + && let ExprKind::Path(QPath::Resolved(None, rhs1_path)) = rhs1.kind + && ident_l.name.as_str() == rhs1_path.segments.iter().map(|el| el.ident.to_string()).collect::>().join("::") + && path_l.segments.iter().map(|el| el.ident.to_string()).collect::>().join("::") == lhs1_path.segments.iter().map(|el| el.ident.to_string()).collect::>().join("::") + { + let lhs1 = Sugg::hir_opt(cx, lhs1); + let rhs1 = Sugg::hir_opt(cx, rhs1); + let (what, lhs, rhs) = if let (Some(first),Some(second)) = (lhs1,rhs1) { + ( + format!(" `{}` and `{}`", first, second), + first.mut_addr().to_string(), + second.mut_addr().to_string(), + ) + } else { + (String::new(), String::new(), String::new()) + }; + let span = first.span.to(second.span); + let Some(sugg) = std_or_core(cx) else { return }; + + lint_almost_swapped_note(span, what, sugg, lhs, rhs); + } } } diff --git a/tests/ui/almost_swapped.rs b/tests/ui/almost_swapped.rs new file mode 100644 index 00000000000..8b0740c3209 --- /dev/null +++ b/tests/ui/almost_swapped.rs @@ -0,0 +1,32 @@ +#![allow(clippy::needless_late_init, clippy::manual_swap)] +#![allow(unused_variables, unused_assignments)] +#![warn(clippy::almost_swapped)] + +fn main() { + let b = 1; + let a = b; + let b = a; + + let mut c = 1; + let mut d = 2; + d = c; + c = d; + + let mut b = 1; + let a = b; + b = a; + + let b = 1; + let a = 2; + + let t = b; + let b = a; + let a = t; + + let mut b = 1; + let mut a = 2; + + let t = b; + b = a; + a = t; +} diff --git a/tests/ui/almost_swapped.stderr b/tests/ui/almost_swapped.stderr new file mode 100644 index 00000000000..70788d23f16 --- /dev/null +++ b/tests/ui/almost_swapped.stderr @@ -0,0 +1,30 @@ +error: this looks like you are trying to swap `a` and `b` + --> $DIR/almost_swapped.rs:7:5 + | +LL | / let a = b; +LL | | let b = a; + | |______________^ + | + = note: `-D clippy::almost-swapped` implied by `-D warnings` + = note: maybe you could use `std::mem::swap(&mut a, &mut b)` or `std::mem::replace`? + +error: this looks like you are trying to swap `d` and `c` + --> $DIR/almost_swapped.rs:12:5 + | +LL | / d = c; +LL | | c = d; + | |_________^ help: try: `std::mem::swap(&mut d, &mut c)` + | + = note: or maybe you should use `std::mem::replace`? + +error: this looks like you are trying to swap `b` and `a` + --> $DIR/almost_swapped.rs:16:5 + | +LL | / let a = b; +LL | | b = a; + | |_________^ + | + = note: maybe you could use `std::mem::swap(&mut b, &mut a)` or `std::mem::replace`? + +error: aborting due to 3 previous errors +