]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/swap.rs
rustup https://github.com/rust-lang/rust/pull/67455
[rust.git] / clippy_lints / src / swap.rs
index 32a67f3943993608729ecd1eb302aff3d2b57e46..f47945f0a14692956c8a9d12e917998be9164497 100644 (file)
@@ -1,14 +1,17 @@
 use crate::utils::sugg::Sugg;
 use crate::utils::{
-    differing_macro_contexts, 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;
+use rustc::declare_lint_pass;
 use rustc::hir::*;
 use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
 use rustc::ty;
-use rustc::{declare_tool_lint, lint_array};
 use rustc_errors::Applicability;
+use rustc_session::declare_tool_lint;
+use syntax_pos::Symbol;
 
 declare_clippy_lint! {
     /// **What it does:** Checks for manual swapping.
     ///
     /// **Example:**
     /// ```rust
+    /// let mut a = 42;
+    /// let mut b = 1337;
+    ///
     /// let t = b;
     /// b = a;
     /// a = t;
     /// ```
     /// Use std::mem::swap():
     /// ```rust
+    /// let mut a = 1;
+    /// let mut b = 2;
     /// std::mem::swap(&mut a, &mut b);
     /// ```
     pub MANUAL_SWAP,
     /// a = b;
     /// b = a;
     /// ```
+    /// Could be written as:
+    /// ```rust
+    /// # let mut a = 1;
+    /// # let mut b = 2;
+    /// std::mem::swap(&mut a, &mut b);
+    /// ```
     pub ALMOST_SWAPPED,
     correctness,
     "`foo = bar; bar = foo` sequence"
 }
 
-#[derive(Copy, Clone)]
-pub struct Swap;
-
-impl LintPass for Swap {
-    fn get_lints(&self) -> LintArray {
-        lint_array![MANUAL_SWAP, ALMOST_SWAPPED]
-    }
-
-    fn name(&self) -> &'static str {
-        "Swap"
-    }
-}
+declare_lint_pass!(Swap => [MANUAL_SWAP, ALMOST_SWAPPED]);
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Swap {
     fn check_block(&mut self, cx: &LateContext<'a, 'tcx>, block: &'tcx Block) {
@@ -77,99 +80,150 @@ fn check_manual_swap(cx: &LateContext<'_, '_>, block: &Block) {
     for w in block.stmts.windows(3) {
         if_chain! {
             // let t = foo();
-            if let StmtKind::Local(ref tmp) = w[0].node;
+            if let StmtKind::Local(ref tmp) = w[0].kind;
             if let Some(ref tmp_init) = tmp.init;
-            if let PatKind::Binding(.., ident, None) = tmp.pat.node;
+            if let PatKind::Binding(.., ident, None) = tmp.pat.kind;
 
             // foo() = bar();
-            if let StmtKind::Semi(ref first) = w[1].node;
-            if let ExprKind::Assign(ref lhs1, ref rhs1) = first.node;
+            if let StmtKind::Semi(ref first) = w[1].kind;
+            if let ExprKind::Assign(ref lhs1, ref rhs1) = first.kind;
 
             // bar() = t;
-            if let StmtKind::Semi(ref second) = w[2].node;
-            if let ExprKind::Assign(ref lhs2, ref rhs2) = second.node;
-            if let ExprKind::Path(QPath::Resolved(None, ref rhs2)) = rhs2.node;
+            if let StmtKind::Semi(ref second) = w[2].kind;
+            if let ExprKind::Assign(ref lhs2, ref rhs2) = second.kind;
+            if let ExprKind::Path(QPath::Resolved(None, ref rhs2)) = rhs2.kind;
             if rhs2.segments.len() == 1;
 
             if ident.as_str() == rhs2.segments[0].ident.as_str();
             if SpanlessEq::new(cx).ignore_fn().eq_expr(tmp_init, lhs1);
             if SpanlessEq::new(cx).ignore_fn().eq_expr(rhs1, lhs2);
             then {
-                fn check_for_slice<'a>(
-                    cx: &LateContext<'_, '_>,
-                    lhs1: &'a Expr,
-                    lhs2: &'a Expr,
-                ) -> Option<(&'a Expr, &'a Expr, &'a Expr)> {
-                    if let ExprKind::Index(ref lhs1, ref idx1) = lhs1.node {
-                        if let ExprKind::Index(ref lhs2, ref idx2) = lhs2.node {
-                            if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs1, lhs2) {
-                                let ty = walk_ptrs_ty(cx.tables.expr_ty(lhs1));
-
-                                if matches!(ty.sty, ty::Slice(_)) ||
-                                    matches!(ty.sty, ty::Array(_, _)) ||
-                                    match_type(cx, ty, &paths::VEC) ||
-                                    match_type(cx, ty, &paths::VEC_DEQUE) {
-                                        return Some((lhs1, idx1, idx2));
-                                }
-                            }
+                if let ExprKind::Field(ref lhs1, _) = lhs1.kind {
+                    if let ExprKind::Field(ref lhs2, _) = lhs2.kind {
+                        if lhs1.hir_id.owner_def_id() == lhs2.hir_id.owner_def_id() {
+                            return;
                         }
                     }
-
-                    None
                 }
 
-                let (replace, what, sugg) = if let Some((slice, idx1, idx2)) = 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 {
                     if let Some(slice) = Sugg::hir_opt(cx, slice) {
-                        (false,
-                         format!(" elements of `{}`", slice),
-                         format!("{}.swap({}, {})",
-                                 slice.maybe_par(),
-                                 snippet(cx, idx1.span, ".."),
-                                 snippet(cx, idx2.span, "..")))
+                        (
+                            false,
+                            format!(" elements of `{}`", slice),
+                            format!(
+                                "{}.swap({}, {})",
+                                slice.maybe_par(),
+                                snippet_with_applicability(cx, idx1.span, "..", &mut applicability),
+                                snippet_with_applicability(cx, idx2.span, "..", &mut applicability),
+                            ),
+                        )
                     } else {
                         (false, String::new(), String::new())
                     }
                 } else if let (Some(first), Some(second)) = (Sugg::hir_opt(cx, lhs1), Sugg::hir_opt(cx, rhs1)) {
-                    (true, format!(" `{}` and `{}`", first, second),
-                        format!("std::mem::swap({}, {})", first.mut_addr(), second.mut_addr()))
+                    (
+                        true,
+                        format!(" `{}` and `{}`", first, second),
+                        format!("std::mem::swap({}, {})", first.mut_addr(), second.mut_addr()),
+                    )
                 } else {
                     (true, String::new(), String::new())
                 };
 
                 let span = w[0].span.to(second.span);
 
-                span_lint_and_then(cx,
-                                   MANUAL_SWAP,
-                                   span,
-                                   &format!("this looks like you are swapping{} manually", what),
-                                   |db| {
-                                       if !sugg.is_empty() {
-                                           db.span_suggestion(
-                                               span,
-                                               "try",
-                                               sugg,
-                                               Applicability::Unspecified,
-                                           );
+                span_lint_and_then(
+                    cx,
+                    MANUAL_SWAP,
+                    span,
+                    &format!("this looks like you are swapping{} manually", what),
+                    |db| {
+                        if !sugg.is_empty() {
+                            db.span_suggestion(
+                                span,
+                                "try",
+                                sugg,
+                                applicability,
+                            );
 
-                                           if replace {
-                                               db.note("or maybe you should use `std::mem::replace`?");
-                                           }
-                                       }
-                                   });
+                            if replace {
+                                db.note("or maybe you should use `std::mem::replace`?");
+                            }
+                        }
+                    }
+                );
             }
         }
     }
 }
 
+enum Slice<'a> {
+    /// `slice.swap(idx1, idx2)` can be used
+    ///
+    /// ## Example
+    ///
+    /// ```rust
+    /// # let mut a = vec![0, 1];
+    /// let t = a[1];
+    /// a[1] = a[0];
+    /// a[0] = t;
+    /// // can be written as
+    /// a.swap(0, 1);
+    /// ```
+    Swappable(&'a Expr, &'a Expr, &'a Expr),
+    /// The `swap` function cannot be used.
+    ///
+    /// ## Example
+    ///
+    /// ```rust
+    /// # let mut a = [vec![1, 2], vec![3, 4]];
+    /// let t = a[0][1];
+    /// a[0][1] = a[1][0];
+    /// a[1][0] = t;
+    /// ```
+    NotSwappable,
+    /// Not a slice
+    None,
+}
+
+/// Checks if both expressions are index operations into "slice-like" types.
+fn check_for_slice<'a>(cx: &LateContext<'_, '_>, lhs1: &'a Expr, lhs2: &'a Expr) -> Slice<'a> {
+    if let ExprKind::Index(ref lhs1, ref idx1) = lhs1.kind {
+        if let ExprKind::Index(ref lhs2, ref idx2) = lhs2.kind {
+            if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs1, lhs2) {
+                let ty = walk_ptrs_ty(cx.tables.expr_ty(lhs1));
+
+                if matches!(ty.kind, ty::Slice(_))
+                    || matches!(ty.kind, ty::Array(_, _))
+                    || is_type_diagnostic_item(cx, ty, Symbol::intern("vec_type"))
+                    || match_type(cx, ty, &paths::VEC_DEQUE)
+                {
+                    return Slice::Swappable(lhs1, idx1, idx2);
+                }
+            } else {
+                return Slice::NotSwappable;
+            }
+        }
+    }
+
+    Slice::None
+}
+
 /// 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(ref first) = w[0].node;
-            if let StmtKind::Semi(ref second) = w[1].node;
+            if let StmtKind::Semi(ref first) = w[0].kind;
+            if let StmtKind::Semi(ref second) = w[1].kind;
             if !differing_macro_contexts(first.span, second.span);
-            if let ExprKind::Assign(ref lhs0, ref rhs0) = first.node;
-            if let ExprKind::Assign(ref lhs1, ref rhs1) = second.node;
+            if let ExprKind::Assign(ref lhs0, ref rhs0) = first.kind;
+            if let ExprKind::Assign(ref lhs1, ref rhs1) = second.kind;
             if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs0, rhs1);
             if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs1, rhs0);
             then {