Merge pull request #2747 from rust-lang-nursery/author

Patterns, locals and matches for author lint

.gitignore

@@ -4,10 +4,12 @@ out
 
 # Compiled files
 *.o
+*.d
 *.so
 *.rlib
 *.dll
 *.pyc
+*.rmeta
 
 # Executables
 *.exe

clippy_lints/src/utils/author.rs

@@ -5,7 +5,7 @@
 
 use rustc::lint::*;
 use rustc::hir;
-use rustc::hir::{Expr, Expr_, QPath, Ty_};
+use rustc::hir::{Expr, Expr_, QPath, Ty_, Pat, PatKind, BindingAnnotation, StmtSemi, StmtExpr, StmtDecl, Decl_, Stmt};
 use rustc::hir::intravisit::{NestedVisitorMap, Visitor};
 use syntax::ast::{self, Attribute, LitKind, DUMMY_NODE_ID};
 use std::collections::HashMap;
@@ -322,10 +322,29 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor {
                 self.current = body_pat;
                 self.visit_block(body);
             },
-            Expr_::ExprMatch(ref _expr, ref _arms, desugaring) => {
+            Expr_::ExprMatch(ref expr, ref arms, desugaring) => {
                 let des = desugaring_name(desugaring);
-                println!("Match(ref expr, ref arms, {}) = {};", des, current);
-                println!("    // unimplemented: `ExprMatch` is not further destructured at the moment");
+                let expr_pat = self.next("expr");
+                let arms_pat = self.next("arms");
+                println!("Match(ref {}, ref {}, {}) = {};", expr_pat, arms_pat, des, current);
+                self.current = expr_pat;
+                self.visit_expr(expr);
+                println!("    if {}.len() == {};", arms_pat, arms.len());
+                for (i, arm) in arms.iter().enumerate() {
+                    self.current = format!("{}[{}].body", arms_pat, i);
+                    self.visit_expr(&arm.body);
+                    if let Some(ref guard) = arm.guard {
+                        let guard_pat = self.next("guard");
+                        println!("    if let Some(ref {}) = {}[{}].guard", guard_pat, arms_pat, i);
+                        self.current = guard_pat;
+                        self.visit_expr(guard);
+                    }
+                    println!("    if {}[{}].pats.len() == {};", arms_pat, i, arm.pats.len());
+                    for (j, pat) in arm.pats.iter().enumerate() {
+                        self.current = format!("{}[{}].pats[{}]", arms_pat, i, j);
+                        self.visit_pat(pat);
+                    }
+                }
             },
             Expr_::ExprClosure(ref _capture_clause, ref _func, _, _, _) => {
                 println!("Closure(ref capture_clause, ref func, _, _, _) = {};", current);
@@ -454,6 +473,160 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor {
         }
     }
 
+    fn visit_pat(&mut self, pat: &Pat) {
+        print!("    if let PatKind::");
+        let current = format!("{}.node", self.current);
+        match pat.node {
+            PatKind::Wild => println!("Wild = {};", current),
+            PatKind::Binding(anno, _, name, ref sub) => {
+                let anno_pat = match anno {
+                    BindingAnnotation::Unannotated => "BindingAnnotation::Unannotated",
+                    BindingAnnotation::Mutable => "BindingAnnotation::Mutable",
+                    BindingAnnotation::Ref => "BindingAnnotation::Ref",
+                    BindingAnnotation::RefMut => "BindingAnnotation::RefMut",
+                };
+                let name_pat = self.next("name");
+                if let Some(ref sub) = *sub {
+                    let sub_pat = self.next("sub");
+                    println!("Binding({}, _, {}, Some(ref {})) = {};", anno_pat, name_pat, sub_pat, current);
+                    self.current = sub_pat;
+                    self.visit_pat(sub);
+                } else {
+                    println!("Binding({}, _, {}, None) = {};", anno_pat, name_pat, current);
+                }
+                println!("    if {}.node.as_str() == \"{}\";", name_pat, name.node.as_str());
+            }
+            PatKind::Struct(ref path, ref fields, ignore) => {
+                let path_pat = self.next("path");
+                let fields_pat = self.next("fields");
+                println!("Struct(ref {}, ref {}, {}) = {};", path_pat, fields_pat, ignore, current);
+                self.current = path_pat;
+                self.print_qpath(path);
+                println!("    if {}.len() == {};", fields_pat, fields.len());
+                println!("    // unimplemented: field checks");
+            }
+            PatKind::TupleStruct(ref path, ref fields, skip_pos) => {
+                let path_pat = self.next("path");
+                let fields_pat = self.next("fields");
+                println!("TupleStruct(ref {}, ref {}, {:?}) = {};", path_pat, fields_pat, skip_pos, current);
+                self.current = path_pat;
+                self.print_qpath(path);
+                println!("    if {}.len() == {};", fields_pat, fields.len());
+                println!("    // unimplemented: field checks");
+            },
+            PatKind::Path(ref path) => {
+                let path_pat = self.next("path");
+                println!("Path(ref {}) = {};", path_pat, current);
+                self.current = path_pat;
+                self.print_qpath(path);
+            }
+            PatKind::Tuple(ref fields, skip_pos) => {
+                let fields_pat = self.next("fields");
+                println!("Tuple(ref {}, {:?}) = {};", fields_pat, skip_pos, current);
+                println!("    if {}.len() == {};", fields_pat, fields.len());
+                println!("    // unimplemented: field checks");
+            }
+            PatKind::Box(ref pat) => {
+                let pat_pat = self.next("pat");
+                println!("Box(ref {}) = {};", pat_pat, current);
+                self.current = pat_pat;
+                self.visit_pat(pat);
+            },
+            PatKind::Ref(ref pat, muta) => {
+                let pat_pat = self.next("pat");
+                println!("Ref(ref {}, Mutability::{:?}) = {};", pat_pat, muta, current);
+                self.current = pat_pat;
+                self.visit_pat(pat);
+            },
+            PatKind::Lit(ref lit_expr) => {
+                let lit_expr_pat = self.next("lit_expr");
+                println!("Lit(ref {}) = {}", lit_expr_pat, current);
+                self.current = lit_expr_pat;
+                self.visit_expr(lit_expr);
+            }
+            PatKind::Range(ref start, ref end, end_kind) => {
+                let start_pat = self.next("start");
+                let end_pat = self.next("end");
+                println!("Range(ref {}, ref {}, RangeEnd::{:?}) = {};", start_pat, end_pat, end_kind, current);
+                self.current = start_pat;
+                self.visit_expr(start);
+                self.current = end_pat;
+                self.visit_expr(end);
+            }
+            PatKind::Slice(ref start, ref middle, ref end) => {
+                let start_pat = self.next("start");
+                let end_pat = self.next("end");
+                if let Some(ref middle) = middle {
+                    let middle_pat = self.next("middle");
+                    println!("Slice(ref {}, Some(ref {}), ref {}) = {};", start_pat, middle_pat, end_pat, current);
+                    self.current = middle_pat;
+                    self.visit_pat(middle);
+                } else {
+                    println!("Slice(ref {}, None, ref {}) = {};", start_pat, end_pat, current);
+                }
+                println!("    if {}.len() == {};", start_pat, start.len());
+                for (i, pat) in start.iter().enumerate() {
+                    self.current = format!("{}[{}]", start_pat, i);
+                    self.visit_pat(pat);
+                }
+                println!("    if {}.len() == {};", end_pat, end.len());
+                for (i, pat) in end.iter().enumerate() {
+                    self.current = format!("{}[{}]", end_pat, i);
+                    self.visit_pat(pat);
+                }
+            }
+        }
+    }
+
+    fn visit_stmt(&mut self, s: &Stmt) {
+        print!("    if let Stmt_::");
+        let current = format!("{}.node", self.current);
+        match s.node {
+            // Could be an item or a local (let) binding:
+            StmtDecl(ref decl, _) => {
+                let decl_pat = self.next("decl");
+                println!("StmtDecl(ref {}, _) = {}", decl_pat, current);
+                print!("    if let Decl_::");
+                let current = format!("{}.node", decl_pat);
+                match decl.node {
+                    // A local (let) binding:
+                    Decl_::DeclLocal(ref local) => {
+                        let local_pat = self.next("local");
+                        println!("DeclLocal(ref {}) = {};", local_pat, current);
+                        if let Some(ref init) = local.init {
+                            let init_pat = self.next("init");
+                            println!("    if let Some(ref {}) = {}.init", init_pat, local_pat);
+                            self.current = init_pat;
+                            self.visit_expr(init);
+                        }
+                        self.current = format!("{}.pat", local_pat);
+                        self.visit_pat(&local.pat);
+                    },
+                    // An item binding:
+                    Decl_::DeclItem(_) => {
+                        println!("DeclItem(item_id) = {};", current);
+                    },
+                }
+            }
+
+            // Expr without trailing semi-colon (must have unit type):
+            StmtExpr(ref e, _) => {
+                let e_pat = self.next("e");
+                println!("StmtExpr(ref {}, _) = {}", e_pat, current);
+                self.current = e_pat;
+                self.visit_expr(e);
+            },
+
+            // Expr with trailing semi-colon (may have any type):
+            StmtSemi(ref e, _) => {
+                let e_pat = self.next("e");
+                println!("StmtSemi(ref {}, _) = {}", e_pat, current);
+                self.current = e_pat;
+                self.visit_expr(e);
+            },
+        }
+    }
+
     fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
         NestedVisitorMap::None
     }

tests/ui/author.stdout

@@ -1,9 +1,14 @@
 if_chain! {
-    if let Expr_::ExprCast(ref expr, ref cast_ty) = stmt.node;
+    if let Stmt_::StmtDecl(ref decl, _) = stmt.node
+    if let Decl_::DeclLocal(ref local) = decl.node;
+    if let Some(ref init) = local.init
+    if let Expr_::ExprCast(ref expr, ref cast_ty) = init.node;
     if let Ty_::TyPath(ref qp) = cast_ty.node;
     if match_qpath(qp, &["char"]);
     if let Expr_::ExprLit(ref lit) = expr.node;
     if let LitKind::Int(69, _) = lit.node;
+    if let PatKind::Binding(BindingAnnotation::Unannotated, _, name, None) = local.pat.node;
+    if name.node.as_str() == "x";
     then {
         // report your lint here
     }

tests/ui/author/for_loop.rs

@@ -0,0 +1,8 @@
+#![feature(custom_attribute)]
+
+fn main() {
+    #[clippy(author)]
+    for y in 0..10 {
+        let z = y;
+    }
+}

tests/ui/author/for_loop.stdout

@@ -0,0 +1,60 @@
+if_chain! {
+    if let Expr_::ExprBlock(ref block) = expr.node;
+    if let Stmt_::StmtDecl(ref decl, _) = block.node
+    if let Decl_::DeclLocal(ref local) = decl.node;
+    if let Some(ref init) = local.init
+    if let Expr_::ExprMatch(ref expr, ref arms, MatchSource::ForLoopDesugar) = init.node;
+    if let Expr_::ExprCall(ref func, ref args) = expr.node;
+    // unimplemented: `ExprCall` is not further destructured at the moment
+    if arms.len() == 1;
+    if let Expr_::ExprLoop(ref body, ref label, LoopSource::ForLoop) = arms[0].body.node;
+    if let Stmt_::StmtDecl(ref decl1, _) = body.node
+    if let Decl_::DeclLocal(ref local1) = decl1.node;
+    if let PatKind::Binding(BindingAnnotation::Mutable, _, name, None) = local1.pat.node;
+    if name.node.as_str() == "__next";
+    if let Stmt_::StmtExpr(ref e, _) = local1.pat.node
+    if let Expr_::ExprMatch(ref expr1, ref arms1, MatchSource::ForLoopDesugar) = e.node;
+    if let Expr_::ExprCall(ref func, ref args) = expr1.node;
+    // unimplemented: `ExprCall` is not further destructured at the moment
+    if arms1.len() == 2;
+    if let Expr_::ExprAssign(ref target, ref value) = arms1[0].body.node;
+    if let Expr_::ExprPath(ref path) = target.node;
+    if match_qpath(path, &["__next"]);
+    if let Expr_::ExprPath(ref path1) = value.node;
+    if match_qpath(path1, &["val"]);
+    if arms1[0].pats.len() == 1;
+    if let PatKind::TupleStruct(ref path2, ref fields, None) = arms1[0].pats[0].node;
+    if match_qpath(path2, &["{{root}}", "std", "option", "Option", "Some"]);
+    if fields.len() == 1;
+    // unimplemented: field checks
+    if let Expr_::ExprBreak(ref destination, None) = arms1[1].body.node;
+    if arms1[1].pats.len() == 1;
+    if let PatKind::Path(ref path3) = arms1[1].pats[0].node;
+    if match_qpath(path3, &["{{root}}", "std", "option", "Option", "None"]);
+    if let Stmt_::StmtDecl(ref decl2, _) = path3.node
+    if let Decl_::DeclLocal(ref local2) = decl2.node;
+    if let Some(ref init1) = local2.init
+    if let Expr_::ExprPath(ref path4) = init1.node;
+    if match_qpath(path4, &["__next"]);
+    if let PatKind::Binding(BindingAnnotation::Unannotated, _, name1, None) = local2.pat.node;
+    if name1.node.as_str() == "y";
+    if let Stmt_::StmtExpr(ref e1, _) = local2.pat.node
+    if let Expr_::ExprBlock(ref block1) = e1.node;
+    if let Stmt_::StmtDecl(ref decl3, _) = block1.node
+    if let Decl_::DeclLocal(ref local3) = decl3.node;
+    if let Some(ref init2) = local3.init
+    if let Expr_::ExprPath(ref path5) = init2.node;
+    if match_qpath(path5, &["y"]);
+    if let PatKind::Binding(BindingAnnotation::Unannotated, _, name2, None) = local3.pat.node;
+    if name2.node.as_str() == "z";
+    if arms[0].pats.len() == 1;
+    if let PatKind::Binding(BindingAnnotation::Mutable, _, name3, None) = arms[0].pats[0].node;
+    if name3.node.as_str() == "iter";
+    if let PatKind::Binding(BindingAnnotation::Unannotated, _, name4, None) = local.pat.node;
+    if name4.node.as_str() == "_result";
+    if let Expr_::ExprPath(ref path6) = local.pat.node;
+    if match_qpath(path6, &["_result"]);
+    then {
+        // report your lint here
+    }
+}

tests/ui/author/matches.rs

@@ -0,0 +1,13 @@
+#![feature(custom_attribute)]
+
+fn main() {
+    #[clippy(author)]
+    let a = match 42 {
+        16 => 5,
+        17 => {
+            let x = 3;
+            x
+        },
+        _ => 1,
+    };
+}

tests/ui/author/matches.stderr

@@ -0,0 +1,15 @@
+error: returning the result of a let binding from a block. Consider returning the expression directly.
+ --> $DIR/matches.rs:9:13
+  |
+9 |             x
+  |             ^
+  |
+  = note: `-D let-and-return` implied by `-D warnings`
+note: this expression can be directly returned
+ --> $DIR/matches.rs:8:21
+  |
+8 |             let x = 3;
+  |                     ^
+
+error: aborting due to previous error
+

tests/ui/author/matches.stout

@@ -0,0 +1,38 @@
+if_chain! {
+    if let Stmt_::StmtDecl(ref decl, _) = stmt.node
+    if let Decl_::DeclLocal(ref local) = decl.node;
+    if let Some(ref init) = local.init
+    if let Expr_::ExprMatch(ref expr, ref arms, MatchSource::Normal) = init.node;
+    if let Expr_::ExprLit(ref lit) = expr.node;
+    if let LitKind::Int(42, _) = lit.node;
+    if arms.len() == 3;
+    if let Expr_::ExprLit(ref lit1) = arms[0].body.node;
+    if let LitKind::Int(5, _) = lit1.node;
+    if arms[0].pats.len() == 1;
+    if let PatKind::Lit(ref lit_expr) = arms[0].pats[0].node
+    if let Expr_::ExprLit(ref lit2) = lit_expr.node;
+    if let LitKind::Int(16, _) = lit2.node;
+    if let Expr_::ExprBlock(ref block) = arms[1].body.node;
+    if let Stmt_::StmtDecl(ref decl1, _) = block.node
+    if let Decl_::DeclLocal(ref local1) = decl1.node;
+    if let Some(ref init1) = local1.init
+    if let Expr_::ExprLit(ref lit3) = init1.node;
+    if let LitKind::Int(3, _) = lit3.node;
+    if let PatKind::Binding(BindingAnnotation::Unannotated, _, name, None) = local1.pat.node;
+    if name.node.as_str() == "x";
+    if let Expr_::ExprPath(ref path) = local1.pat.node;
+    if match_qpath(path, &["x"]);
+    if arms[1].pats.len() == 1;
+    if let PatKind::Lit(ref lit_expr1) = arms[1].pats[0].node
+    if let Expr_::ExprLit(ref lit4) = lit_expr1.node;
+    if let LitKind::Int(17, _) = lit4.node;
+    if let Expr_::ExprLit(ref lit5) = arms[2].body.node;
+    if let LitKind::Int(1, _) = lit5.node;
+    if arms[2].pats.len() == 1;
+    if let PatKind::Wild = arms[2].pats[0].node;
+    if let PatKind::Binding(BindingAnnotation::Unannotated, _, name1, None) = local.pat.node;
+    if name1.node.as_str() == "a";
+    then {
+        // report your lint here
+    }
+}

tests/ui/for_loop.rs

@@ -14,7 +14,7 @@ fn for_loop_over_option_and_result() {
     let v = vec![0, 1, 2];
 
     // check FOR_LOOP_OVER_OPTION lint
-    #[clippy(author)]for x in option {
+    for x in option {
         println!("{}", x);
     }
 

tests/ui/for_loop.stderr

@@ -1,8 +1,8 @@
 error: for loop over `option`, which is an `Option`. This is more readably written as an `if let` statement.
-  --> $DIR/for_loop.rs:17:31
+  --> $DIR/for_loop.rs:17:14
    |
-17 |     #[clippy(author)]for x in option {
-   |                               ^^^^^^
+17 |     for x in option {
+   |              ^^^^^^
    |
    = note: `-D for-loop-over-option` implied by `-D warnings`
    = help: consider replacing `for x in option` with `if let Some(x) = option`

tests/ui/for_loop.stdout

@@ -1,20 +0,0 @@
-if_chain! {
-    if let Expr_::ExprBlock(ref block) = stmt.node;
-    if let Expr_::ExprMatch(ref expr, ref arms, MatchSource::ForLoopDesugar) = block.node;
-    // unimplemented: `ExprMatch` is not further destructured at the moment
-    if let Expr_::ExprPath(ref path) = block.node;
-    if match_qpath(path, &["_result"]);
-    then {
-        // report your lint here
-    }
-}
-if_chain! {
-    if let Expr_::ExprBlock(ref block) = expr.node;
-    if let Expr_::ExprMatch(ref expr, ref arms, MatchSource::ForLoopDesugar) = block.node;
-    // unimplemented: `ExprMatch` is not further destructured at the moment
-    if let Expr_::ExprPath(ref path) = block.node;
-    if match_qpath(path, &["_result"]);
-    then {
-        // report your lint here
-    }
-}