diff options
| -rw-r--r-- | clippy_lints/src/swap.rs | 167 | ||||
| -rw-r--r-- | tests/ui/almost_swapped.rs | 32 | ||||
| -rw-r--r-- | tests/ui/almost_swapped.stderr | 30 | ||||
| -rw-r--r-- | tests/ui/swap.fixed | 24 | ||||
| -rw-r--r-- | tests/ui/swap.rs | 29 | ||||
| -rw-r--r-- | tests/ui/swap.stderr | 65 |
6 files changed, 159 insertions, 188 deletions
diff --git a/clippy_lints/src/swap.rs b/clippy_lints/src/swap.rs index 5c408a73a55..0f062cecf88 100644 --- a/clippy_lints/src/swap.rs +++ b/clippy_lints/src/swap.rs @@ -10,7 +10,7 @@ use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Spanned; -use rustc_span::{sym, Span}; +use rustc_span::{sym, symbol::Ident, Span}; declare_clippy_lint! { /// ### What it does @@ -172,129 +172,76 @@ 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) { - if_chain! { - if let StmtKind::Semi(first) = w[0].kind; - if let StmtKind::Semi(second) = w[1].kind; - if first.span.ctxt() == second.span.ctxt(); - if let ExprKind::Assign(lhs0, rhs0, _) = first.kind; - if let ExprKind::Assign(lhs1, rhs1, _) = second.kind; - if eq_expr_value(cx, lhs0, rhs1); - if eq_expr_value(cx, lhs1, rhs0); - then { - let lhs0 = Sugg::hir_opt(cx, lhs0); - let rhs0 = Sugg::hir_opt(cx, rhs0); - let (what, lhs, rhs) = if let (Some(first), Some(second)) = (lhs0, rhs0) { - ( - format!(" `{first}` and `{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 }; - - span_lint_and_then(cx, - ALMOST_SWAPPED, - span, - &format!("this looks like you are trying to swap{what}"), - |diag| { - if !what.is_empty() { - diag.span_suggestion( - span, - "try", - format!( - "{sugg}::mem::swap({lhs}, {rhs})", - ), - Applicability::MaybeIncorrect, - ); - diag.note( - format!("or maybe you should use `{sugg}::mem::replace`?") - ); - } - }); + for [first, second] in block.stmts.array_windows() { + if let Some((lhs0, rhs0)) = parse(first) + && let Some((lhs1, rhs1)) = parse(second) + && first.span.eq_ctxt(second.span) + && is_same(cx, lhs0, rhs1) + && is_same(cx, lhs1, rhs0) + && let Some(lhs_sugg) = match &lhs0 { + ExprOrIdent::Expr(expr) => Sugg::hir_opt(cx, expr), + ExprOrIdent::Ident(ident) => Some(Sugg::NonParen(ident.as_str().into())), } - } - - let lint_almost_swapped_note = |span, what: String, sugg, lhs, rhs| { + && let Some(rhs_sugg) = Sugg::hir_opt(cx, rhs0) + { + let span = first.span.to(rhs1.span); + let Some(sugg) = std_or_core(cx) else { return }; span_lint_and_then( cx, ALMOST_SWAPPED, span, - &format!("this looks like you are trying to swap{}", what), + &format!("this looks like you are trying to swap `{lhs_sugg}` and `{rhs_sugg}`"), |diag| { - if !what.is_empty() { - diag.note(&format!( - "maybe you could use `{sugg}::mem::swap({lhs}, {rhs})` or `{sugg}::mem::replace`?" - )); - } + diag.span_suggestion( + span, + "try", + format!("{sugg}::mem::swap({}, {})", lhs_sugg.mut_addr(), rhs_sugg.mut_addr()), + Applicability::MaybeIncorrect, + ); + diag.note(format!("or maybe you should use `{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::<Vec<_>>().join("::") - && ident_r.name.as_str() == path_l.segments.iter().map(|el| el.ident.to_string()).collect::<Vec<_>>().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); - } +fn is_same(cx: &LateContext<'_>, lhs: ExprOrIdent<'_>, rhs: &Expr<'_>) -> bool { + match lhs { + ExprOrIdent::Expr(expr) => eq_expr_value(cx, expr, rhs), + ExprOrIdent::Ident(ident) => { + if let ExprKind::Path(QPath::Resolved(None, path)) = rhs.kind + && let [segment] = &path.segments + && segment.ident == ident + { + true + } else { + false + } + } + } +} - 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::<Vec<_>>().join("::") - && path_l.segments.iter().map(|el| el.ident.to_string()).collect::<Vec<_>>().join("::") == lhs1_path.segments.iter().map(|el| el.ident.to_string()).collect::<Vec<_>>().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 }; +#[derive(Debug, Clone, Copy)] +enum ExprOrIdent<'a> { + Expr(&'a Expr<'a>), + Ident(Ident), +} - lint_almost_swapped_note(span, what, sugg, lhs, rhs); - } +fn parse<'a, 'hir>(stmt: &'a Stmt<'hir>) -> Option<(ExprOrIdent<'hir>, &'a Expr<'hir>)> { + if let StmtKind::Semi(expr) = stmt.kind { + if let ExprKind::Assign(lhs, rhs, _) = expr.kind { + return Some((ExprOrIdent::Expr(lhs), rhs)); + } + } else if let StmtKind::Local(expr) = stmt.kind { + if let Some(rhs) = expr.init { + if let PatKind::Binding(_, _, ident_l, _) = expr.pat.kind { + return Some((ExprOrIdent::Ident(ident_l), rhs)); + } + } } + None } /// Implementation of the xor case for `MANUAL_SWAP` lint. diff --git a/tests/ui/almost_swapped.rs b/tests/ui/almost_swapped.rs deleted file mode 100644 index 8b0740c3209..00000000000 --- a/tests/ui/almost_swapped.rs +++ /dev/null @@ -1,32 +0,0 @@ -#![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 deleted file mode 100644 index 70788d23f16..00000000000 --- a/tests/ui/almost_swapped.stderr +++ /dev/null @@ -1,30 +0,0 @@ -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 - diff --git a/tests/ui/swap.fixed b/tests/ui/swap.fixed index 805a2ba5a59..fa89706a815 100644 --- a/tests/ui/swap.fixed +++ b/tests/ui/swap.fixed @@ -7,7 +7,8 @@ clippy::redundant_clone, redundant_semicolons, dead_code, - unused_assignments + unused_assignments, + unused_variables )] struct Foo(u32); @@ -121,6 +122,27 @@ fn main() { std::mem::swap(&mut c.0, &mut a); ; std::mem::swap(&mut c.0, &mut a); + + std::mem::swap(&mut a, &mut b); + + let mut c = 1; + let mut d = 2; + std::mem::swap(&mut d, &mut c); + + let mut b = 1; + std::mem::swap(&mut a, &mut b); + + let b = 1; + let a = 2; + + let t = b; + let b = a; + let a = t; + + let mut b = 1; + let mut a = 2; + + std::mem::swap(&mut b, &mut a); } fn issue_8154() { diff --git a/tests/ui/swap.rs b/tests/ui/swap.rs index a8c87847952..ef8a81c8341 100644 --- a/tests/ui/swap.rs +++ b/tests/ui/swap.rs @@ -7,7 +7,8 @@ clippy::redundant_clone, redundant_semicolons, dead_code, - unused_assignments + unused_assignments, + unused_variables )] struct Foo(u32); @@ -143,6 +144,32 @@ fn main() { ; let t = c.0; c.0 = a; a = t; + + 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; } fn issue_8154() { diff --git a/tests/ui/swap.stderr b/tests/ui/swap.stderr index ee4b7a508a5..f0acbfe253f 100644 --- a/tests/ui/swap.stderr +++ b/tests/ui/swap.stderr @@ -1,5 +1,5 @@ error: this looks like you are swapping `bar.a` and `bar.b` manually - --> $DIR/swap.rs:24:5 + --> $DIR/swap.rs:25:5 | LL | / let temp = bar.a; LL | | bar.a = bar.b; @@ -10,7 +10,7 @@ LL | | bar.b = temp; = note: `-D clippy::manual-swap` implied by `-D warnings` error: this looks like you are swapping elements of `foo` manually - --> $DIR/swap.rs:36:5 + --> $DIR/swap.rs:37:5 | LL | / let temp = foo[0]; LL | | foo[0] = foo[1]; @@ -18,7 +18,7 @@ LL | | foo[1] = temp; | |_________________^ help: try: `foo.swap(0, 1)` error: this looks like you are swapping elements of `foo` manually - --> $DIR/swap.rs:45:5 + --> $DIR/swap.rs:46:5 | LL | / let temp = foo[0]; LL | | foo[0] = foo[1]; @@ -26,7 +26,7 @@ LL | | foo[1] = temp; | |_________________^ help: try: `foo.swap(0, 1)` error: this looks like you are swapping elements of `foo` manually - --> $DIR/swap.rs:64:5 + --> $DIR/swap.rs:65:5 | LL | / let temp = foo[0]; LL | | foo[0] = foo[1]; @@ -34,7 +34,7 @@ LL | | foo[1] = temp; | |_________________^ help: try: `foo.swap(0, 1)` error: this looks like you are swapping `a` and `b` manually - --> $DIR/swap.rs:75:5 + --> $DIR/swap.rs:76:5 | LL | / a ^= b; LL | | b ^= a; @@ -42,7 +42,7 @@ LL | | a ^= b; | |___________^ help: try: `std::mem::swap(&mut a, &mut b)` error: this looks like you are swapping `bar.a` and `bar.b` manually - --> $DIR/swap.rs:83:5 + --> $DIR/swap.rs:84:5 | LL | / bar.a ^= bar.b; LL | | bar.b ^= bar.a; @@ -50,7 +50,7 @@ LL | | bar.a ^= bar.b; | |___________________^ help: try: `std::mem::swap(&mut bar.a, &mut bar.b)` error: this looks like you are swapping elements of `foo` manually - --> $DIR/swap.rs:91:5 + --> $DIR/swap.rs:92:5 | LL | / foo[0] ^= foo[1]; LL | | foo[1] ^= foo[0]; @@ -58,7 +58,7 @@ LL | | foo[0] ^= foo[1]; | |_____________________^ help: try: `foo.swap(0, 1)` error: this looks like you are swapping `foo[0][1]` and `bar[1][0]` manually - --> $DIR/swap.rs:120:5 + --> $DIR/swap.rs:121:5 | LL | / let temp = foo[0][1]; LL | | foo[0][1] = bar[1][0]; @@ -68,7 +68,7 @@ LL | | bar[1][0] = temp; = note: or maybe you should use `std::mem::replace`? error: this looks like you are swapping `a` and `b` manually - --> $DIR/swap.rs:134:7 + --> $DIR/swap.rs:135:7 | LL | ; let t = a; | _______^ @@ -79,7 +79,7 @@ LL | | b = t; = note: or maybe you should use `std::mem::replace`? error: this looks like you are swapping `c.0` and `a` manually - --> $DIR/swap.rs:143:7 + --> $DIR/swap.rs:144:7 | LL | ; let t = c.0; | _______^ @@ -89,8 +89,18 @@ LL | | a = t; | = note: or maybe you should use `std::mem::replace`? +error: this looks like you are swapping `b` and `a` manually + --> $DIR/swap.rs:170:5 + | +LL | / let t = b; +LL | | b = a; +LL | | a = t; + | |_________^ help: try: `std::mem::swap(&mut b, &mut a)` + | + = note: or maybe you should use `std::mem::replace`? + error: this looks like you are trying to swap `a` and `b` - --> $DIR/swap.rs:131:5 + --> $DIR/swap.rs:132:5 | LL | / a = b; LL | | b = a; @@ -100,7 +110,7 @@ LL | | b = a; = note: `-D clippy::almost-swapped` implied by `-D warnings` error: this looks like you are trying to swap `c.0` and `a` - --> $DIR/swap.rs:140:5 + --> $DIR/swap.rs:141:5 | LL | / c.0 = a; LL | | a = c.0; @@ -108,8 +118,35 @@ LL | | a = c.0; | = note: or maybe you should use `std::mem::replace`? +error: this looks like you are trying to swap `a` and `b` + --> $DIR/swap.rs:148:5 + | +LL | / let a = b; +LL | | let b = a; + | |_____________^ help: try: `std::mem::swap(&mut a, &mut b)` + | + = note: or maybe you should use `std::mem::replace`? + +error: this looks like you are trying to swap `d` and `c` + --> $DIR/swap.rs:153: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 `a` and `b` + --> $DIR/swap.rs:157:5 + | +LL | / let a = b; +LL | | b = a; + | |_________^ help: try: `std::mem::swap(&mut a, &mut b)` + | + = note: or maybe you should use `std::mem::replace`? + error: this looks like you are swapping `s.0.x` and `s.0.y` manually - --> $DIR/swap.rs:178:5 + --> $DIR/swap.rs:205:5 | LL | / let t = s.0.x; LL | | s.0.x = s.0.y; @@ -118,5 +155,5 @@ LL | | s.0.y = t; | = note: or maybe you should use `std::mem::replace`? -error: aborting due to 13 previous errors +error: aborting due to 17 previous errors |