Lint `foo = bar; bar = foo` sequences

diff --git a/README.md b/README.md
index c9730c4c58b6d186249cff0111240697e363d815..f7527372585a8a47d94169a2b3140999dd3a8467 100644 (file)
--- a/README.md
+++ b/README.md
@@ -8,7 +8,7 @@ A collection of lints to catch common mistakes and improve your Rust code.
 [Jump to usage instructions](#usage)
 
 ##Lints
-There are 125 lints included in this crate:
+There are 126 lints included in this crate:
 
 name                                                                                                                 | default | meaning
 ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -114,6 +114,7 @@ name
 [string_to_string](https://github.com/Manishearth/rust-clippy/wiki#string_to_string)                                 | warn    | calling `String::to_string` which is inefficient
 [suspicious_assignment_formatting](https://github.com/Manishearth/rust-clippy/wiki#suspicious_assignment_formatting) | warn    | suspicious formatting of `*=`, `-=` or `!=`
 [suspicious_else_formatting](https://github.com/Manishearth/rust-clippy/wiki#suspicious_else_formatting)             | warn    | suspicious formatting of `else if`
+[suspicious_swap](https://github.com/Manishearth/rust-clippy/wiki#suspicious_swap)                                   | warn    | `foo = bar; bar = foo` sequence
 [temporary_assignment](https://github.com/Manishearth/rust-clippy/wiki#temporary_assignment)                         | warn    | assignments to temporaries
 [toplevel_ref_arg](https://github.com/Manishearth/rust-clippy/wiki#toplevel_ref_arg)                                 | warn    | An entire binding was declared as `ref`, in a function argument (`fn foo(ref x: Bar)`), or a `let` statement (`let ref x = foo()`). In such cases, it is preferred to take references with `&`.
 [trivial_regex](https://github.com/Manishearth/rust-clippy/wiki#trivial_regex)                                       | warn    | finds trivial regular expressions in `Regex::new(_)` invocations

diff --git a/src/lib.rs b/src/lib.rs
index 47d9fc6f24c50da0809cff61fdd91aff8c719312..217110e2bd8482640207dc9daedfeee1f20566f9 100644 (file)
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -86,6 +86,7 @@ fn main() {
 pub mod returns;
 pub mod shadow;
 pub mod strings;
+pub mod swap;
 pub mod temporary_assignment;
 pub mod transmute;
 pub mod types;
@@ -167,6 +168,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_late_lint_pass(box copies::CopyAndPaste);
     reg.register_late_lint_pass(box format::FormatMacLint);
     reg.register_early_lint_pass(box formatting::Formatting);
+    reg.register_late_lint_pass(box swap::Swap);
 
     reg.register_lint_group("clippy_pedantic", vec![
         enum_glob_use::ENUM_GLOB_USE,
@@ -285,6 +287,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
         returns::LET_AND_RETURN,
         returns::NEEDLESS_RETURN,
         strings::STRING_LIT_AS_BYTES,
+        swap::SUSPICIOUS_SWAP,
         temporary_assignment::TEMPORARY_ASSIGNMENT,
         transmute::USELESS_TRANSMUTE,
         types::ABSURD_EXTREME_COMPARISONS,

diff --git a/src/swap.rs b/src/swap.rs
new file mode 100644 (file)
index 0000000..8e1b1e7
--- /dev/null
+++ b/src/swap.rs
@@ -0,0 +1,66 @@
+use rustc::lint::*;
+use rustc_front::hir::*;
+use utils::{differing_macro_contexts, snippet_opt, span_lint_and_then, SpanlessEq};
+use syntax::codemap::mk_sp;
+
+/// **What it does:** This lints `foo = bar; bar = foo` sequences.
+///
+/// **Why is this bad?** This looks like a failed attempt to swap.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust,ignore
+/// a = b;
+/// b = a;
+/// ```
+declare_lint! {
+    pub SUSPICIOUS_SWAP,
+    Warn,
+    "`foo = bar; bar = foo` sequence"
+}
+
+#[derive(Copy,Clone)]
+pub struct Swap;
+
+impl LintPass for Swap {
+    fn get_lints(&self) -> LintArray {
+        lint_array![SUSPICIOUS_SWAP]
+    }
+}
+
+impl LateLintPass for Swap {
+    fn check_block(&mut self, cx: &LateContext, block: &Block) {
+        for w in block.stmts.windows(2) {
+            if_let_chain!{[
+                let StmtSemi(ref first, _) = w[0].node,
+                let StmtSemi(ref second, _) = w[1].node,
+                !differing_macro_contexts(first.span, second.span),
+                let ExprAssign(ref lhs0, ref rhs0) = first.node,
+                let ExprAssign(ref lhs1, ref rhs1) = second.node,
+                SpanlessEq::new(cx).ignore_fn().eq_expr(lhs0, rhs1),
+                SpanlessEq::new(cx).ignore_fn().eq_expr(lhs1, rhs0)
+            ], {
+                let (what, lhs, rhs) = if let (Some(first), Some(second)) = (snippet_opt(cx, lhs0.span), snippet_opt(cx, rhs0.span)) {
+                    (format!(" `{}` and `{}`", first, second), first, second)
+                } else {
+                    ("".to_owned(), "".to_owned(), "".to_owned())
+                };
+
+                let span = mk_sp(first.span.lo, second.span.hi);
+
+                span_lint_and_then(cx,
+                                   SUSPICIOUS_SWAP,
+                                   span,
+                                   &format!("this looks like you are trying to swap{}", what),
+                                   |db| {
+                                       if !what.is_empty() {
+                                           db.span_suggestion(span, "try",
+                                                              format!("std::mem::swap({}, {})", lhs, rhs));
+                                           db.fileline_note(span, "or maybe you should use `std::mem::replace`?");
+                                       }
+                                   });
+            }}
+        }
+    }
+}

diff --git a/tests/compile-fail/swap.rs b/tests/compile-fail/swap.rs
new file mode 100755 (executable)
index 0000000..be2c785
--- /dev/null
+++ b/tests/compile-fail/swap.rs
@@ -0,0 +1,17 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+
+#![deny(clippy)]
+#![allow(unused_assignments)]
+
+fn main() {
+    let mut a = 42;
+    let mut b = 1337;
+
+    a = b;
+    b = a;
+    //~^^ ERROR this looks like you are trying to swap `a` and `b`
+    //~| HELP try
+    //~| SUGGESTION std::mem::swap(a, b);
+    //~| NOTE or maybe you should use `std::mem::replace`?
+}