Skip to content

Commit 553b90b

Browse files
oli-obkoli-obk
authored
Merge pull request #2747 from rust-lang-nursery/author
Patterns, locals and matches for author lint
2 parents cd3d22d + 22bef4c commit 553b90b

File tree

11 files changed

+323
-29
lines changed

11 files changed

+323
-29
lines changed

‎.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@ out
44

55
# Compiled files
66
*.o
7+
*.d
78
*.so
89
*.rlib
910
*.dll
1011
*.pyc
12+
*.rmeta
1113

1214
# Executables
1315
*.exe

‎clippy_lints/src/utils/author.rs

Lines changed: 177 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
use rustc::lint::*;
77
use rustc::hir;
8-
use rustc::hir::{Expr, Expr_, QPath, Ty_};
8+
use rustc::hir::{Expr, Expr_, QPath, Ty_, Pat, PatKind, BindingAnnotation, StmtSemi, StmtExpr, StmtDecl, Decl_, Stmt};
99
use rustc::hir::intravisit::{NestedVisitorMap, Visitor};
1010
use syntax::ast::{self, Attribute, LitKind, DUMMY_NODE_ID};
1111
use std::collections::HashMap;
@@ -322,10 +322,29 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor {
322322
self.current = body_pat;
323323
self.visit_block(body);
324324
},
325-
Expr_::ExprMatch(ref _expr, ref _arms, desugaring) => {
325+
Expr_::ExprMatch(ref expr, ref arms, desugaring) => {
326326
let des = desugaring_name(desugaring);
327-
println!("Match(ref expr, ref arms, {}) = {};", des, current);
328-
println!(" // unimplemented: `ExprMatch` is not further destructured at the moment");
327+
let expr_pat = self.next("expr");
328+
let arms_pat = self.next("arms");
329+
println!("Match(ref {}, ref {}, {}) = {};", expr_pat, arms_pat, des, current);
330+
self.current = expr_pat;
331+
self.visit_expr(expr);
332+
println!(" if {}.len() == {};", arms_pat, arms.len());
333+
for (i, arm) in arms.iter().enumerate() {
334+
self.current = format!("{}[{}].body", arms_pat, i);
335+
self.visit_expr(&arm.body);
336+
if let Some(ref guard) = arm.guard {
337+
let guard_pat = self.next("guard");
338+
println!(" if let Some(ref {}) = {}[{}].guard", guard_pat, arms_pat, i);
339+
self.current = guard_pat;
340+
self.visit_expr(guard);
341+
}
342+
println!(" if {}[{}].pats.len() == {};", arms_pat, i, arm.pats.len());
343+
for (j, pat) in arm.pats.iter().enumerate() {
344+
self.current = format!("{}[{}].pats[{}]", arms_pat, i, j);
345+
self.visit_pat(pat);
346+
}
347+
}
329348
},
330349
Expr_::ExprClosure(ref _capture_clause, ref _func, _, _, _) => {
331350
println!("Closure(ref capture_clause, ref func, _, _, _) = {};", current);
@@ -454,6 +473,160 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor {
454473
}
455474
}
456475

476+
fn visit_pat(&mut self, pat: &Pat) {
477+
print!(" if let PatKind::");
478+
let current = format!("{}.node", self.current);
479+
match pat.node {
480+
PatKind::Wild => println!("Wild = {};", current),
481+
PatKind::Binding(anno, _, name, ref sub) => {
482+
let anno_pat = match anno {
483+
BindingAnnotation::Unannotated => "BindingAnnotation::Unannotated",
484+
BindingAnnotation::Mutable => "BindingAnnotation::Mutable",
485+
BindingAnnotation::Ref => "BindingAnnotation::Ref",
486+
BindingAnnotation::RefMut => "BindingAnnotation::RefMut",
487+
};
488+
let name_pat = self.next("name");
489+
if let Some(ref sub) = *sub {
490+
let sub_pat = self.next("sub");
491+
println!("Binding({}, _, {}, Some(ref {})) = {};", anno_pat, name_pat, sub_pat, current);
492+
self.current = sub_pat;
493+
self.visit_pat(sub);
494+
} else {
495+
println!("Binding({}, _, {}, None) = {};", anno_pat, name_pat, current);
496+
}
497+
println!(" if {}.node.as_str() == \"{}\";", name_pat, name.node.as_str());
498+
}
499+
PatKind::Struct(ref path, ref fields, ignore) => {
500+
let path_pat = self.next("path");
501+
let fields_pat = self.next("fields");
502+
println!("Struct(ref {}, ref {}, {}) = {};", path_pat, fields_pat, ignore, current);
503+
self.current = path_pat;
504+
self.print_qpath(path);
505+
println!(" if {}.len() == {};", fields_pat, fields.len());
506+
println!(" // unimplemented: field checks");
507+
}
508+
PatKind::TupleStruct(ref path, ref fields, skip_pos) => {
509+
let path_pat = self.next("path");
510+
let fields_pat = self.next("fields");
511+
println!("TupleStruct(ref {}, ref {}, {:?}) = {};", path_pat, fields_pat, skip_pos, current);
512+
self.current = path_pat;
513+
self.print_qpath(path);
514+
println!(" if {}.len() == {};", fields_pat, fields.len());
515+
println!(" // unimplemented: field checks");
516+
},
517+
PatKind::Path(ref path) => {
518+
let path_pat = self.next("path");
519+
println!("Path(ref {}) = {};", path_pat, current);
520+
self.current = path_pat;
521+
self.print_qpath(path);
522+
}
523+
PatKind::Tuple(ref fields, skip_pos) => {
524+
let fields_pat = self.next("fields");
525+
println!("Tuple(ref {}, {:?}) = {};", fields_pat, skip_pos, current);
526+
println!(" if {}.len() == {};", fields_pat, fields.len());
527+
println!(" // unimplemented: field checks");
528+
}
529+
PatKind::Box(ref pat) => {
530+
let pat_pat = self.next("pat");
531+
println!("Box(ref {}) = {};", pat_pat, current);
532+
self.current = pat_pat;
533+
self.visit_pat(pat);
534+
},
535+
PatKind::Ref(ref pat, muta) => {
536+
let pat_pat = self.next("pat");
537+
println!("Ref(ref {}, Mutability::{:?}) = {};", pat_pat, muta, current);
538+
self.current = pat_pat;
539+
self.visit_pat(pat);
540+
},
541+
PatKind::Lit(ref lit_expr) => {
542+
let lit_expr_pat = self.next("lit_expr");
543+
println!("Lit(ref {}) = {}", lit_expr_pat, current);
544+
self.current = lit_expr_pat;
545+
self.visit_expr(lit_expr);
546+
}
547+
PatKind::Range(ref start, ref end, end_kind) => {
548+
let start_pat = self.next("start");
549+
let end_pat = self.next("end");
550+
println!("Range(ref {}, ref {}, RangeEnd::{:?}) = {};", start_pat, end_pat, end_kind, current);
551+
self.current = start_pat;
552+
self.visit_expr(start);
553+
self.current = end_pat;
554+
self.visit_expr(end);
555+
}
556+
PatKind::Slice(ref start, ref middle, ref end) => {
557+
let start_pat = self.next("start");
558+
let end_pat = self.next("end");
559+
if let Some(ref middle) = middle {
560+
let middle_pat = self.next("middle");
561+
println!("Slice(ref {}, Some(ref {}), ref {}) = {};", start_pat, middle_pat, end_pat, current);
562+
self.current = middle_pat;
563+
self.visit_pat(middle);
564+
} else {
565+
println!("Slice(ref {}, None, ref {}) = {};", start_pat, end_pat, current);
566+
}
567+
println!(" if {}.len() == {};", start_pat, start.len());
568+
for (i, pat) in start.iter().enumerate() {
569+
self.current = format!("{}[{}]", start_pat, i);
570+
self.visit_pat(pat);
571+
}
572+
println!(" if {}.len() == {};", end_pat, end.len());
573+
for (i, pat) in end.iter().enumerate() {
574+
self.current = format!("{}[{}]", end_pat, i);
575+
self.visit_pat(pat);
576+
}
577+
}
578+
}
579+
}
580+
581+
fn visit_stmt(&mut self, s: &Stmt) {
582+
print!(" if let Stmt_::");
583+
let current = format!("{}.node", self.current);
584+
match s.node {
585+
// Could be an item or a local (let) binding:
586+
StmtDecl(ref decl, _) => {
587+
let decl_pat = self.next("decl");
588+
println!("StmtDecl(ref {}, _) = {}", decl_pat, current);
589+
print!(" if let Decl_::");
590+
let current = format!("{}.node", decl_pat);
591+
match decl.node {
592+
// A local (let) binding:
593+
Decl_::DeclLocal(ref local) => {
594+
let local_pat = self.next("local");
595+
println!("DeclLocal(ref {}) = {};", local_pat, current);
596+
if let Some(ref init) = local.init {
597+
let init_pat = self.next("init");
598+
println!(" if let Some(ref {}) = {}.init", init_pat, local_pat);
599+
self.current = init_pat;
600+
self.visit_expr(init);
601+
}
602+
self.current = format!("{}.pat", local_pat);
603+
self.visit_pat(&local.pat);
604+
},
605+
// An item binding:
606+
Decl_::DeclItem(_) => {
607+
println!("DeclItem(item_id) = {};", current);
608+
},
609+
}
610+
}
611+
612+
// Expr without trailing semi-colon (must have unit type):
613+
StmtExpr(ref e, _) => {
614+
let e_pat = self.next("e");
615+
println!("StmtExpr(ref {}, _) = {}", e_pat, current);
616+
self.current = e_pat;
617+
self.visit_expr(e);
618+
},
619+
620+
// Expr with trailing semi-colon (may have any type):
621+
StmtSemi(ref e, _) => {
622+
let e_pat = self.next("e");
623+
println!("StmtSemi(ref {}, _) = {}", e_pat, current);
624+
self.current = e_pat;
625+
self.visit_expr(e);
626+
},
627+
}
628+
}
629+
457630
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
458631
NestedVisitorMap::None
459632
}

‎tests/ui/author.stdout

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
if_chain! {
2-
if let Expr_::ExprCast(ref expr, ref cast_ty) = stmt.node;
2+
if let Stmt_::StmtDecl(ref decl, _) = stmt.node
3+
if let Decl_::DeclLocal(ref local) = decl.node;
4+
if let Some(ref init) = local.init
5+
if let Expr_::ExprCast(ref expr, ref cast_ty) = init.node;
36
if let Ty_::TyPath(ref qp) = cast_ty.node;
47
if match_qpath(qp, &["char"]);
58
if let Expr_::ExprLit(ref lit) = expr.node;
69
if let LitKind::Int(69, _) = lit.node;
10+
if let PatKind::Binding(BindingAnnotation::Unannotated, _, name, None) = local.pat.node;
11+
if name.node.as_str() == "x";
712
then {
813
// report your lint here
914
}

‎tests/ui/author/for_loop.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
#![feature(custom_attribute)]
2+
3+
fn main() {
4+
#[clippy(author)]
5+
for y in 0..10 {
6+
let z = y;
7+
}
8+
}

‎tests/ui/author/for_loop.stdout

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
if_chain! {
2+
if let Expr_::ExprBlock(ref block) = expr.node;
3+
if let Stmt_::StmtDecl(ref decl, _) = block.node
4+
if let Decl_::DeclLocal(ref local) = decl.node;
5+
if let Some(ref init) = local.init
6+
if let Expr_::ExprMatch(ref expr, ref arms, MatchSource::ForLoopDesugar) = init.node;
7+
if let Expr_::ExprCall(ref func, ref args) = expr.node;
8+
// unimplemented: `ExprCall` is not further destructured at the moment
9+
if arms.len() == 1;
10+
if let Expr_::ExprLoop(ref body, ref label, LoopSource::ForLoop) = arms[0].body.node;
11+
if let Stmt_::StmtDecl(ref decl1, _) = body.node
12+
if let Decl_::DeclLocal(ref local1) = decl1.node;
13+
if let PatKind::Binding(BindingAnnotation::Mutable, _, name, None) = local1.pat.node;
14+
if name.node.as_str() == "__next";
15+
if let Stmt_::StmtExpr(ref e, _) = local1.pat.node
16+
if let Expr_::ExprMatch(ref expr1, ref arms1, MatchSource::ForLoopDesugar) = e.node;
17+
if let Expr_::ExprCall(ref func, ref args) = expr1.node;
18+
// unimplemented: `ExprCall` is not further destructured at the moment
19+
if arms1.len() == 2;
20+
if let Expr_::ExprAssign(ref target, ref value) = arms1[0].body.node;
21+
if let Expr_::ExprPath(ref path) = target.node;
22+
if match_qpath(path, &["__next"]);
23+
if let Expr_::ExprPath(ref path1) = value.node;
24+
if match_qpath(path1, &["val"]);
25+
if arms1[0].pats.len() == 1;
26+
if let PatKind::TupleStruct(ref path2, ref fields, None) = arms1[0].pats[0].node;
27+
if match_qpath(path2, &["{{root}}", "std", "option", "Option", "Some"]);
28+
if fields.len() == 1;
29+
// unimplemented: field checks
30+
if let Expr_::ExprBreak(ref destination, None) = arms1[1].body.node;
31+
if arms1[1].pats.len() == 1;
32+
if let PatKind::Path(ref path3) = arms1[1].pats[0].node;
33+
if match_qpath(path3, &["{{root}}", "std", "option", "Option", "None"]);
34+
if let Stmt_::StmtDecl(ref decl2, _) = path3.node
35+
if let Decl_::DeclLocal(ref local2) = decl2.node;
36+
if let Some(ref init1) = local2.init
37+
if let Expr_::ExprPath(ref path4) = init1.node;
38+
if match_qpath(path4, &["__next"]);
39+
if let PatKind::Binding(BindingAnnotation::Unannotated, _, name1, None) = local2.pat.node;
40+
if name1.node.as_str() == "y";
41+
if let Stmt_::StmtExpr(ref e1, _) = local2.pat.node
42+
if let Expr_::ExprBlock(ref block1) = e1.node;
43+
if let Stmt_::StmtDecl(ref decl3, _) = block1.node
44+
if let Decl_::DeclLocal(ref local3) = decl3.node;
45+
if let Some(ref init2) = local3.init
46+
if let Expr_::ExprPath(ref path5) = init2.node;
47+
if match_qpath(path5, &["y"]);
48+
if let PatKind::Binding(BindingAnnotation::Unannotated, _, name2, None) = local3.pat.node;
49+
if name2.node.as_str() == "z";
50+
if arms[0].pats.len() == 1;
51+
if let PatKind::Binding(BindingAnnotation::Mutable, _, name3, None) = arms[0].pats[0].node;
52+
if name3.node.as_str() == "iter";
53+
if let PatKind::Binding(BindingAnnotation::Unannotated, _, name4, None) = local.pat.node;
54+
if name4.node.as_str() == "_result";
55+
if let Expr_::ExprPath(ref path6) = local.pat.node;
56+
if match_qpath(path6, &["_result"]);
57+
then {
58+
// report your lint here
59+
}
60+
}

‎tests/ui/author/matches.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
#![feature(custom_attribute)]
2+
3+
fn main() {
4+
#[clippy(author)]
5+
let a = match 42 {
6+
16 => 5,
7+
17 => {
8+
let x = 3;
9+
x
10+
},
11+
_ => 1,
12+
};
13+
}

‎tests/ui/author/matches.stderr

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
error: returning the result of a let binding from a block. Consider returning the expression directly.
2+
--> $DIR/matches.rs:9:13
3+
|
4+
9 | x
5+
| ^
6+
|
7+
= note: `-D let-and-return` implied by `-D warnings`
8+
note: this expression can be directly returned
9+
--> $DIR/matches.rs:8:21
10+
|
11+
8 | let x = 3;
12+
| ^
13+
14+
error: aborting due to previous error
15+

‎tests/ui/author/matches.stout

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
if_chain! {
2+
if let Stmt_::StmtDecl(ref decl, _) = stmt.node
3+
if let Decl_::DeclLocal(ref local) = decl.node;
4+
if let Some(ref init) = local.init
5+
if let Expr_::ExprMatch(ref expr, ref arms, MatchSource::Normal) = init.node;
6+
if let Expr_::ExprLit(ref lit) = expr.node;
7+
if let LitKind::Int(42, _) = lit.node;
8+
if arms.len() == 3;
9+
if let Expr_::ExprLit(ref lit1) = arms[0].body.node;
10+
if let LitKind::Int(5, _) = lit1.node;
11+
if arms[0].pats.len() == 1;
12+
if let PatKind::Lit(ref lit_expr) = arms[0].pats[0].node
13+
if let Expr_::ExprLit(ref lit2) = lit_expr.node;
14+
if let LitKind::Int(16, _) = lit2.node;
15+
if let Expr_::ExprBlock(ref block) = arms[1].body.node;
16+
if let Stmt_::StmtDecl(ref decl1, _) = block.node
17+
if let Decl_::DeclLocal(ref local1) = decl1.node;
18+
if let Some(ref init1) = local1.init
19+
if let Expr_::ExprLit(ref lit3) = init1.node;
20+
if let LitKind::Int(3, _) = lit3.node;
21+
if let PatKind::Binding(BindingAnnotation::Unannotated, _, name, None) = local1.pat.node;
22+
if name.node.as_str() == "x";
23+
if let Expr_::ExprPath(ref path) = local1.pat.node;
24+
if match_qpath(path, &["x"]);
25+
if arms[1].pats.len() == 1;
26+
if let PatKind::Lit(ref lit_expr1) = arms[1].pats[0].node
27+
if let Expr_::ExprLit(ref lit4) = lit_expr1.node;
28+
if let LitKind::Int(17, _) = lit4.node;
29+
if let Expr_::ExprLit(ref lit5) = arms[2].body.node;
30+
if let LitKind::Int(1, _) = lit5.node;
31+
if arms[2].pats.len() == 1;
32+
if let PatKind::Wild = arms[2].pats[0].node;
33+
if let PatKind::Binding(BindingAnnotation::Unannotated, _, name1, None) = local.pat.node;
34+
if name1.node.as_str() == "a";
35+
then {
36+
// report your lint here
37+
}
38+
}

‎tests/ui/for_loop.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ fn for_loop_over_option_and_result() {
1414
let v = vec![0, 1, 2];
1515

1616
// check FOR_LOOP_OVER_OPTION lint
17-
#[clippy(author)]for x in option {
17+
for x in option {
1818
println!("{}", x);
1919
}
2020

‎tests/ui/for_loop.stderr

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
error: for loop over `option`, which is an `Option`. This is more readably written as an `if let` statement.
2-
--> $DIR/for_loop.rs:17:31
2+
--> $DIR/for_loop.rs:17:14
33
|
4-
17 | #[clippy(author)]for x in option {
5-
| ^^^^^^
4+
17 | for x in option {
5+
| ^^^^^^
66
|
77
= note: `-D for-loop-over-option` implied by `-D warnings`
88
= help: consider replacing `for x in option` with `if let Some(x) = option`

0 commit comments

Comments
 (0)