]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/let_if_seq.rs
Merge branch 'macro-use' into HEAD
[rust.git] / clippy_lints / src / let_if_seq.rs
index 88d74b01886335e69e58edbee8009119acaa78b1..57ca5eff95530b8646dc5dd14ac2f0214c9376e4 100644 (file)
@@ -1,7 +1,11 @@
 use rustc::lint::*;
+use rustc::{declare_lint, lint_array};
+use if_chain::if_chain;
 use rustc::hir;
-use syntax::codemap;
-use utils::{snippet, span_lint_and_then};
+use rustc::hir::BindingAnnotation;
+use rustc::hir::def::Def;
+use syntax::ast;
+use crate::utils::{snippet, span_lint_and_then};
 
 /// **What it does:** Checks for variable declarations immediately followed by a
 /// conditional affectation.
 ///     None
 /// };
 /// ```
-declare_lint! {
+declare_clippy_lint! {
     pub USELESS_LET_IF_SEQ,
-    Warn,
+    style,
     "unidiomatic `let mut` declaration followed by initialization in `if`"
 }
 
-#[derive(Copy,Clone)]
+#[derive(Copy, Clone)]
 pub struct LetIfSeq;
 
 impl LintPass for LetIfSeq {
@@ -61,131 +65,137 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetIfSeq {
     fn check_block(&mut self, cx: &LateContext<'a, 'tcx>, block: &'tcx hir::Block) {
         let mut it = block.stmts.iter().peekable();
         while let Some(stmt) = it.next() {
-            if_let_chain! {[
-                let Some(expr) = it.peek(),
-                let hir::StmtDecl(ref decl, _) = stmt.node,
-                let hir::DeclLocal(ref decl) = decl.node,
-                let hir::PatKind::Binding(mode, def_id, ref name, None) = decl.pat.node,
-                let hir::StmtExpr(ref if_, _) = expr.node,
-                let hir::ExprIf(ref cond, ref then, ref else_) = if_.node,
-                !used_in_expr(cx, def_id, cond),
-                let Some(value) = check_assign(cx, def_id, then),
-                !used_in_expr(cx, def_id, value),
-            ], {
-                let span = codemap::mk_sp(stmt.span.lo, if_.span.hi);
-
-                let (default_multi_stmts, default) = if let Some(ref else_) = *else_ {
-                    if let hir::ExprBlock(ref else_) = else_.node {
-                        if let Some(default) = check_assign(cx, def_id, else_) {
-                            (else_.stmts.len() > 1, default)
-                        } else if let Some(ref default) = decl.init {
-                            (true, &**default)
+            if_chain! {
+                if let Some(expr) = it.peek();
+                if let hir::StmtKind::Decl(ref decl, _) = stmt.node;
+                if let hir::DeclKind::Local(ref decl) = decl.node;
+                if let hir::PatKind::Binding(mode, canonical_id, ident, None) = decl.pat.node;
+                if let hir::StmtKind::Expr(ref if_, _) = expr.node;
+                if let hir::ExprKind::If(ref cond, ref then, ref else_) = if_.node;
+                if !used_in_expr(cx, canonical_id, cond);
+                if let hir::ExprKind::Block(ref then, _) = then.node;
+                if let Some(value) = check_assign(cx, canonical_id, &*then);
+                if !used_in_expr(cx, canonical_id, value);
+                then {
+                    let span = stmt.span.to(if_.span);
+
+                    let (default_multi_stmts, default) = if let Some(ref else_) = *else_ {
+                        if let hir::ExprKind::Block(ref else_, _) = else_.node {
+                            if let Some(default) = check_assign(cx, canonical_id, else_) {
+                                (else_.stmts.len() > 1, default)
+                            } else if let Some(ref default) = decl.init {
+                                (true, &**default)
+                            } else {
+                                continue;
+                            }
                         } else {
                             continue;
                         }
+                    } else if let Some(ref default) = decl.init {
+                        (false, &**default)
                     } else {
                         continue;
-                    }
-                } else if let Some(ref default) = decl.init {
-                    (false, &**default)
-                } else {
-                    continue;
-                };
-
-                let mutability = match mode {
-                    hir::BindByRef(hir::MutMutable) | hir::BindByValue(hir::MutMutable) => "<mut> ",
-                    _ => "",
-                };
-
-                // FIXME: this should not suggest `mut` if we can detect that the variable is not
-                // use mutably after the `if`
-
-                let sug = format!(
-                    "let {mut}{name} = if {cond} {{{then} {value} }} else {{{else} {default} }};",
-                    mut=mutability,
-                    name=name.node,
-                    cond=snippet(cx, cond.span, "_"),
-                    then=if then.stmts.len() > 1 { " ..;" } else { "" },
-                    else=if default_multi_stmts { " ..;" } else { "" },
-                    value=snippet(cx, value.span, "<value>"),
-                    default=snippet(cx, default.span, "<default>"),
-                );
-                span_lint_and_then(cx,
-                                   USELESS_LET_IF_SEQ,
-                                   span,
-                                   "`if _ { .. } else { .. }` is an expression",
-                                   |db| {
-                                       db.span_suggestion(span,
-                                                          "it is more idiomatic to write",
-                                                          sug);
-                                       if !mutability.is_empty() {
-                                           db.note("you might not need `mut` at all");
-                                       }
-                                   });
-            }}
+                    };
+
+                    let mutability = match mode {
+                        BindingAnnotation::RefMut | BindingAnnotation::Mutable => "<mut> ",
+                        _ => "",
+                    };
+
+                    // FIXME: this should not suggest `mut` if we can detect that the variable is not
+                    // use mutably after the `if`
+
+                    let sug = format!(
+                        "let {mut}{name} = if {cond} {{{then} {value} }} else {{{else} {default} }};",
+                        mut=mutability,
+                        name=ident.name,
+                        cond=snippet(cx, cond.span, "_"),
+                        then=if then.stmts.len() > 1 { " ..;" } else { "" },
+                        else=if default_multi_stmts { " ..;" } else { "" },
+                        value=snippet(cx, value.span, "<value>"),
+                        default=snippet(cx, default.span, "<default>"),
+                    );
+                    span_lint_and_then(cx,
+                                       USELESS_LET_IF_SEQ,
+                                       span,
+                                       "`if _ { .. } else { .. }` is an expression",
+                                       |db| {
+                                           db.span_suggestion(span,
+                                                              "it is more idiomatic to write",
+                                                              sug);
+                                           if !mutability.is_empty() {
+                                               db.note("you might not need `mut` at all");
+                                           }
+                                       });
+                }
+            }
         }
     }
 }
 
 struct UsedVisitor<'a, 'tcx: 'a> {
     cx: &'a LateContext<'a, 'tcx>,
-    id: hir::def_id::DefId,
+    id: ast::NodeId,
     used: bool,
 }
 
 impl<'a, 'tcx> hir::intravisit::Visitor<'tcx> for UsedVisitor<'a, 'tcx> {
     fn visit_expr(&mut self, expr: &'tcx hir::Expr) {
-        if_let_chain! {[
-            let hir::ExprPath(ref qpath) = expr.node,
-            self.id == self.cx.tcx.tables().qpath_def(qpath, expr.id).def_id(),
-        ], {
-            self.used = true;
-            return;
-        }}
+        if_chain! {
+            if let hir::ExprKind::Path(ref qpath) = expr.node;
+            if let Def::Local(local_id) = self.cx.tables.qpath_def(qpath, expr.hir_id);
+            if self.id == local_id;
+            then {
+                self.used = true;
+                return;
+            }
+        }
         hir::intravisit::walk_expr(self, expr);
     }
     fn nested_visit_map<'this>(&'this mut self) -> hir::intravisit::NestedVisitorMap<'this, 'tcx> {
-        hir::intravisit::NestedVisitorMap::All(&self.cx.tcx.map)
+        hir::intravisit::NestedVisitorMap::None
     }
 }
 
 fn check_assign<'a, 'tcx>(
     cx: &LateContext<'a, 'tcx>,
-    decl: hir::def_id::DefId,
-    block: &'tcx hir::Block
+    decl: ast::NodeId,
+    block: &'tcx hir::Block,
 ) -> Option<&'tcx hir::Expr> {
-    if_let_chain! {[
-        block.expr.is_none(),
-        let Some(expr) = block.stmts.iter().last(),
-        let hir::StmtSemi(ref expr, _) = expr.node,
-        let hir::ExprAssign(ref var, ref value) = expr.node,
-        let hir::ExprPath(ref qpath) = var.node,
-        decl == cx.tcx.tables().qpath_def(qpath, var.id).def_id(),
-    ], {
-        let mut v = UsedVisitor {
-            cx: cx,
-            id: decl,
-            used: false,
-        };
-
-        for s in block.stmts.iter().take(block.stmts.len()-1) {
-            hir::intravisit::walk_stmt(&mut v, s);
-
-            if v.used {
-                return None;
+    if_chain! {
+        if block.expr.is_none();
+        if let Some(expr) = block.stmts.iter().last();
+        if let hir::StmtKind::Semi(ref expr, _) = expr.node;
+        if let hir::ExprKind::Assign(ref var, ref value) = expr.node;
+        if let hir::ExprKind::Path(ref qpath) = var.node;
+        if let Def::Local(local_id) = cx.tables.qpath_def(qpath, var.hir_id);
+        if decl == local_id;
+        then {
+            let mut v = UsedVisitor {
+                cx,
+                id: decl,
+                used: false,
+            };
+
+            for s in block.stmts.iter().take(block.stmts.len()-1) {
+                hir::intravisit::walk_stmt(&mut v, s);
+
+                if v.used {
+                    return None;
+                }
             }
-        }
 
-        return Some(value);
-    }}
+            return Some(value);
+        }
+    }
 
     None
 }
 
-fn used_in_expr<'a, 'tcx: 'a>(cx: &LateContext<'a, 'tcx>, id: hir::def_id::DefId, expr: &'tcx hir::Expr) -> bool {
+fn used_in_expr<'a, 'tcx: 'a>(cx: &LateContext<'a, 'tcx>, id: ast::NodeId, expr: &'tcx hir::Expr) -> bool {
     let mut v = UsedVisitor {
-        cx: cx,
-        id: id,
+        cx,
+        id,
         used: false,
     };
     hir::intravisit::walk_expr(&mut v, expr);