From 1e3b24de4389491dac9ce630668c710df5950c70 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Tue, 3 Dec 2019 13:42:05 +0100 Subject: [PATCH] Make manual_swap autofixable --- clippy_lints/src/swap.rs | 13 ++++--- tests/ui/swap.fixed | 83 ++++++++++++++++++++++++++++++++++++++++ tests/ui/swap.rs | 3 ++ tests/ui/swap.stderr | 14 +++---- 4 files changed, 100 insertions(+), 13 deletions(-) create mode 100644 tests/ui/swap.fixed diff --git a/clippy_lints/src/swap.rs b/clippy_lints/src/swap.rs index bb294c467fa8..77288869ce0e 100644 --- a/clippy_lints/src/swap.rs +++ b/clippy_lints/src/swap.rs @@ -1,7 +1,7 @@ use crate::utils::sugg::Sugg; use crate::utils::{ - differing_macro_contexts, is_type_diagnostic_item, match_type, paths, snippet, span_lint_and_then, walk_ptrs_ty, - SpanlessEq, + differing_macro_contexts, is_type_diagnostic_item, match_type, paths, snippet_with_applicability, + span_lint_and_then, walk_ptrs_ty, SpanlessEq, }; use if_chain::if_chain; use matches::matches; @@ -105,8 +105,9 @@ fn check_manual_swap(cx: &LateContext<'_, '_>, block: &Block) { } } - let slice = check_for_slice(cx, lhs1, lhs2); + let mut applicability = Applicability::MachineApplicable; + let slice = check_for_slice(cx, lhs1, lhs2); let (replace, what, sugg) = if let Slice::NotSwappable = slice { return; } else if let Slice::Swappable(slice, idx1, idx2) = slice { @@ -117,8 +118,8 @@ fn check_manual_swap(cx: &LateContext<'_, '_>, block: &Block) { format!( "{}.swap({}, {})", slice.maybe_par(), - snippet(cx, idx1.span, ".."), - snippet(cx, idx2.span, ".."), + snippet_with_applicability(cx, idx1.span, "..", &mut applicability), + snippet_with_applicability(cx, idx2.span, "..", &mut applicability), ), ) } else { @@ -147,7 +148,7 @@ fn check_manual_swap(cx: &LateContext<'_, '_>, block: &Block) { span, "try", sugg, - Applicability::Unspecified, + applicability, ); if replace { diff --git a/tests/ui/swap.fixed b/tests/ui/swap.fixed new file mode 100644 index 000000000000..82931bf91fdf --- /dev/null +++ b/tests/ui/swap.fixed @@ -0,0 +1,83 @@ +// run-rustfix + +#![warn(clippy::all)] +#![allow( + clippy::blacklisted_name, + clippy::no_effect, + clippy::redundant_clone, + redundant_semicolon, + unused_assignments +)] + +struct Foo(u32); + +#[derive(Clone)] +struct Bar { + a: u32, + b: u32, +} + +fn field() { + let mut bar = Bar { a: 1, b: 2 }; + + let temp = bar.a; + bar.a = bar.b; + bar.b = temp; + + let mut baz = vec![bar.clone(), bar.clone()]; + let temp = baz[0].a; + baz[0].a = baz[1].a; + baz[1].a = temp; +} + +fn array() { + let mut foo = [1, 2]; + foo.swap(0, 1); + + foo.swap(0, 1); +} + +fn slice() { + let foo = &mut [1, 2]; + foo.swap(0, 1); + + foo.swap(0, 1); +} + +fn unswappable_slice() { + let foo = &mut [vec![1, 2], vec![3, 4]]; + let temp = foo[0][1]; + foo[0][1] = foo[1][0]; + foo[1][0] = temp; + + // swap(foo[0][1], foo[1][0]) would fail +} + +fn vec() { + let mut foo = vec![1, 2]; + foo.swap(0, 1); + + foo.swap(0, 1); +} + +#[rustfmt::skip] +fn main() { + field(); + array(); + slice(); + unswappable_slice(); + vec(); + + let mut a = 42; + let mut b = 1337; + + std::mem::swap(&mut a, &mut b); + + ; std::mem::swap(&mut a, &mut b); + + let mut c = Foo(42); + + std::mem::swap(&mut c.0, &mut a); + + ; std::mem::swap(&mut c.0, &mut a); +} diff --git a/tests/ui/swap.rs b/tests/ui/swap.rs index f5e9182bed1e..4582db4b40e2 100644 --- a/tests/ui/swap.rs +++ b/tests/ui/swap.rs @@ -1,3 +1,5 @@ +// run-rustfix + #![warn(clippy::all)] #![allow( clippy::blacklisted_name, @@ -69,6 +71,7 @@ fn main() { field(); array(); slice(); + unswappable_slice(); vec(); let mut a = 42; diff --git a/tests/ui/swap.stderr b/tests/ui/swap.stderr index f3622230042b..f49bcfedf3a1 100644 --- a/tests/ui/swap.stderr +++ b/tests/ui/swap.stderr @@ -1,5 +1,5 @@ error: this looks like you are swapping elements of `foo` manually - --> $DIR/swap.rs:33:5 + --> $DIR/swap.rs:35:5 | LL | / let temp = foo[0]; LL | | foo[0] = foo[1]; @@ -9,7 +9,7 @@ LL | | foo[1] = temp; = note: `-D clippy::manual-swap` implied by `-D warnings` error: this looks like you are swapping elements of `foo` manually - --> $DIR/swap.rs:42:5 + --> $DIR/swap.rs:44:5 | LL | / let temp = foo[0]; LL | | foo[0] = foo[1]; @@ -17,7 +17,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:60:5 + --> $DIR/swap.rs:62:5 | LL | / let temp = foo[0]; LL | | foo[0] = foo[1]; @@ -25,7 +25,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:80:7 + --> $DIR/swap.rs:83:7 | LL | ; let t = a; | _______^ @@ -36,7 +36,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:89:7 + --> $DIR/swap.rs:92:7 | LL | ; let t = c.0; | _______^ @@ -47,7 +47,7 @@ LL | | a = t; = note: or maybe you should use `std::mem::replace`? error: this looks like you are trying to swap `a` and `b` - --> $DIR/swap.rs:77:5 + --> $DIR/swap.rs:80:5 | LL | / a = b; LL | | b = a; @@ -57,7 +57,7 @@ LL | | b = a; = note: or maybe you should use `std::mem::replace`? error: this looks like you are trying to swap `c.0` and `a` - --> $DIR/swap.rs:86:5 + --> $DIR/swap.rs:89:5 | LL | / c.0 = a; LL | | a = c.0;